Last modified: 2007-07-09 21:45:27 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 10316 - javascript variable "skin" defined twice, breaks with &useskin=
javascript variable "skin" defined twice, breaks with &useskin=
Product: MediaWiki
Classification: Unclassified
Parser (Other open bugs)
All All
: High normal with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
: easy
Depends on:
  Show dependency treegraph
Reported: 2007-06-19 21:49 UTC by Splarka
Modified: 2007-07-09 21:45 UTC (History)
1 user (show)

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

Fix for old JS variables declaration (547 bytes, patch)
2007-07-04 08:39 UTC, Łukasz Matysiak

Description Splarka 2007-06-19 21:49:24 UTC
In /includes/Skin.php in the gen=js, "skin" and "stylepath" are each defined twice. Once in the in-page <script> tag, and once in the gen=js handler:

$s .= "var skin = '{$this->skinname}';\nvar stylepath = '{$wgStylePath}';";

As the gen=js script (MediaWiki:Skinname.js and MediaWiki:Common.js) is loaded later, this overwrites the former definition. This causes a problem with the &useskin URL parameter and "skin" variable, as the gen=js script is cached with the "skin" variable defined by the site default or user preference skin choice. This means, that any check for skin, like "if(skin=='monobook')" is doomed to failure when used with &useskin

Live example:

Suggested fix: As the skin definition in gen=js appears to be a legacy issue it could probably simply be removed, along with stylepath. However, for backwards support, a simple if-defined check would work just as well (to check if defined, check if the parent object has such an attribute; for a global variable, this is the window object) :

$s .= "if(! var skin = '{$this->skinname}';\nvar stylepath = '{$wgStylePath}';";
Comment 1 Łukasz Matysiak 2007-07-04 08:39:17 UTC
Created attachment 3869 [details]
Fix for old JS variables declaration

Fix for bug.
Comment 2 Brion Vibber 2007-07-09 20:53:55 UTC
The gen=js also includes the skin-specific JS, such as MediaWiki:Monobook.js,
so any incompatibility would go to much more than that one variable.

A cache-safe fix would be for the gen=js link to include variant information (eg the current skin name) in the URL.
Comment 3 Brion Vibber 2007-07-09 21:45:27 UTC
I've done this in r23924.

Not sure whether the global 'skin' and 'stylepath' settings should be removed from the gen=js or from the inline vars, but this fixes the inconsistency between them.
It also fixes the inconsistent use of skin-specific .js files (MediaWiki:Monobook.js loaded for wrong skin, etc).
By passing the skin name directly in the gen=js, we ensure both that we have the correct skin information cached
and that you'll get the JS along with useskin= on an HTML page.

Normally useskin= prevents caching, but RawPage handles its own caching headers, so this doesn't cause any problems here. Doesn't seem to be a performance problem in my quick ab testing either.

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