Last modified: 2011-01-25 00:44:49 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 T21704, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 19704 - improve/fix CentralNotice style loading
improve/fix CentralNotice style loading
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
CentralNotice (Other open bugs)
unspecified
All All
: Normal trivial (vote)
: ---
Assigned To: Ryan Kaldari
:
: 18250 (view as bug list)
Depends on: 11746
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-14 07:52 UTC by Splarka
Modified: 2011-01-25 00:44 UTC (History)
2 users (show)

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


Attachments

Description Splarka 2009-07-14 07:52:34 UTC
Currently CentralNotice drops a <style> tag in via "document.writeln($encShowStyle);" from an offsite script. In some older browser engines this can arbitrarily drop the style tag almost anywhere in the dom (including the textarea). A document.writeln() should not be performed via offsite script due to the unpredictablility of exactly when it will execute. This should be changed to an appendCSS() call (a core feature in wikibits.js since 1.13, and using proper DOM methods) or such.

Also, is this CSS strictly necessary if the notice has been dismissed or has been overwritten locally? Can this appendCSS call be moved inside the "if(wgNotice != '')" scope below?
Comment 1 Brion Vibber 2009-07-19 17:43:46 UTC
There's no ambiguity here; per HTML spec, the <script> must be executed before any further processing of following HTML source.
Comment 2 Splarka 2009-07-19 20:36:32 UTC
(In reply to comment #1)
> There's no ambiguity here; per HTML spec, the <script> must be executed before
> any further processing of following HTML source.
> 

Fine, but the second point of the bug is still valid. There is no need to generate the CSS if the notice is dismissed.
Comment 3 Brion Vibber 2009-07-19 20:49:22 UTC
There are several issues rolled up into this "bug":

1) Dislike for use of document.write() based on rumor of instability:

There's no ambiguity in behavior when this is used by synchronously-loaded scripts as in current CentralNotice. Only browser reported to have problems is a 6-year old pre-alpha of Mozilla Firebird, and can be quite freshly ignored.


2) The text is also inserted via document.write(), which there's no other general complaint about here... There is however in bug 11746 which appears to have been (mistakenly?) resolved as FIXED.

Adding bug 11746 as a dependency; document.write() is incompatible with the wiki being served out as application/xhtml+xml.


3) Request not to append styles unnecessarily when the item is dismissed.

Collapse/expand or dissmiss/show behavior in CentralNotice are actually *produced* via CSS; there's no inherent notion of a dismissed CentralNotice. (In fact it's highly discouraged; all messages should have a way to return to them, which is why it's designed to support collapsing/expanding primarily.)

Not showing the CSS would simply result in the message being always shown.


4) The insertion of <style> into the document body instead of the header where it belongs in theory. This wasn't specifically mentioned as a reason, but would be resolved if doing an appendCss() call as requested.

Note however that there are two style blocks relevant:
4a) The setup for hiding/showing of components based on cookie state
4b) Notice campaign-specific CSS, defined as a <style> chunk in the campaign template

The latter is not produced by the software, and would continue to be inserted via document.write() (or else via something like innerHTML, but still in the wrong place) unless CentralNotice were changed to have separate HTML body and style sections for each campaign notice.
Comment 4 Brion Vibber 2009-07-19 20:51:45 UTC
I should add another:

5) Loading the notice state via a synchronous inline <script> delays page rendering a bit, which is a bit annoying. If the way things are inserted is changed to be friendly to a synchronous load, we could let the browser render the page more quickly.

On the downside, this could mean the page "jumps" when a banner gets inserted afterwards.
Comment 5 Lupo 2010-01-16 10:34:05 UTC
*** Bug 18250 has been marked as a duplicate of this bug. ***
Comment 6 Ryan Kaldari 2011-01-05 21:56:42 UTC
The document.writeln($encShowStyle) got dropped a few months ago. This should be fixed now.

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


Navigation
Links