Last modified: 2014-07-11 23:45:11 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 T60086, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 58086 - mw.MwEmbedSupport.style should only load on the pages that need it
mw.MwEmbedSupport.style should only load on the pages that need it
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
TimedMediaHandler (Other open bugs)
unspecified
All All
: Normal normal with 1 vote (vote)
: ---
Assigned To: Michael Dale
: easy, performance
Depends on:
Blocks: 58083 55550
  Show dependency treegraph
 
Reported: 2013-12-06 07:57 UTC by Ori Livneh
Modified: 2014-07-11 23:45 UTC (History)
12 users (show)

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


Attachments

Description Ori Livneh 2013-12-06 07:57:03 UTC
'mw.MwEmbedSupport.style' is injected unconditionally into all pages. There's a TODO in MwEmbedSupport.hooks.php for loading it only when it is needed. It is a proper bug, IMO, since it exacts a nontrivial toll on page performance and bandwidth.
Comment 1 MZMcBride 2013-12-09 04:14:35 UTC
Given ResourceLoader, this should be pretty easy, right?
Comment 2 Ori Livneh 2013-12-09 07:28:59 UTC
(In reply to comment #1)
> Given ResourceLoader, this should be pretty easy, right?

The difficulty is correctly identified in a comment in TimedMediaHandler.hooks.php:

		// We should probably move this script output to a parser function but not working correctly in
		// dynamic contexts ( for example in special upload, when there is an "existing file" warning. )

In other words, it's trivial to only include the stylesheet on article pages that contain timed media, but that doesn't cover cases in which a page's content is loaded via AJAX and injected into the current page. There are probably only a few such dynamic cases. My preference -- for this and for the JavaScript payload which accompanies it -- would be to expose them by reverting the load-everywhere workaround, and then wait for the bugs to trickle in.
Comment 3 Bawolff (Brian Wolff) 2013-12-09 19:21:06 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > Given ResourceLoader, this should be pretty easy, right?
> 
> The difficulty is correctly identified in a comment in
> TimedMediaHandler.hooks.php:
> 
>         // We should probably move this script output to a parser function
> but
> not working correctly in
>         // dynamic contexts ( for example in special upload, when there is an
> "existing file" warning. )
> 
> In other words, it's trivial to only include the stylesheet on article pages
> that contain timed media, but that doesn't cover cases in which a page's
> content is loaded via AJAX and injected into the current page. There are
> probably only a few such dynamic cases. My preference -- for this and for the
> JavaScript payload which accompanies it -- would be to expose them by
> reverting
> the load-everywhere workaround, and then wait for the bugs to trickle in.

Special:upload would be the main example. Also possible when fetching file desc from commons on a foreign image (if for example desc includes video but image isn't one)
Comment 4 Nemo 2014-06-10 11:18:49 UTC
(In reply to Ori Livneh from comment #2)
> My preference -- for this and for
> the JavaScript payload which accompanies it -- would be to expose them by
> reverting the load-everywhere workaround, and then wait for the bugs to
> trickle in.

Sounds sensible.
Comment 5 Matthew Flaschen 2014-06-10 20:01:50 UTC
The fix should take care of the clear cases up front (e.g. categories, Special:ListFiles, Special:Upload (the one flagged in the comment itself), Special:UploadWizard).  Any remaining ones that can't be thought of up front can be caught like that.
Comment 6 Bartosz Dziewoński 2014-07-11 21:26:32 UTC
This was resolved by the fixes for bug 55550, no?
Comment 7 Matthew Flaschen 2014-07-11 23:45:11 UTC
It looks like it's no longer being loaded unconditionally; see https://gerrit.wikimedia.org/r/#/c/145584/2/MwEmbedSupport.hooks.php

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


Navigation
Links