Last modified: 2014-08-01 19:52:09 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T54042, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 52042 - jqueryMsg uses the same astCache for all parser objects
jqueryMsg uses the same astCache for all parser objects
Status: NEW
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
1.22.0
All All
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-25 18:32 UTC by Bartosz Dziewoński
Modified: 2014-08-01 19:52 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Bartosz Dziewoński 2013-07-25 18:32:16 UTC
Oh man, this was annoying to discover…

jqueryMsg uses the same astCache for all parser objects. This is apparently expected behavior, but I'm pretty sure it's incorrect, as each parser can have different messages under the same key.


	mw.jqueryMsg.parser.prototype = {
		...
		 * This cache is shared by all instances of mw.jqueryMsg.parser.
		astCache: {},
		...
	}
Comment 1 Neil Kandalgaonkar 2013-07-25 19:14:28 UTC
Seems like this would be easy to fix, just create astCache as a property upon initialization. 

Seems to have been a bug since the beginning (sorry), but the fact that there's a comment makes me think that maybe someone has deliberately preserved this behavior now. I can't think of any situation where you want multiple instances to share a cache, is there? 

If you need multiple newly-created objects to share a cache, write a singleton to wrap this class. Kind of an antipattern IMO but whatever.
Comment 2 Matthew Flaschen 2013-07-26 04:58:42 UTC
I added the comment after I also discovered it the hard way.  If it's desired behavior to allow different messages with the same key, I agree it needs to change.  It would use more memory, but simplify things.  I think this has come up for me when writing unit tests.

If this change is made, onlyCurlyBraceTransform can also be removed from the AST key, since that is a per-parser setting.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links