Page MenuHomePhabricator

Produce/preserve the metadata about additional ResourceLoader modules required by extension tags
Closed, ResolvedPublic


Parsoid must produce/preserve the metadata about additional ResourceLoader modules required by extension tags. (I've heard some gossip that it falls back to PHP parser to do this, so this should be easy to do.)

For example, in PHP parser, "<syntaxhighlight lang=php>$foo</syntaxhighlight>" will add up to two modules to ParserOutput's $mModuleStyles:

  • 'ext.geshi.language.php' (there is such a module for every supported language)
  • 'ext.geshi.local' (if $wgUseSiteCss is true)

This information must somehow be provided by the API used by VisualEditor.

The minimum set of things that have to be provided is the ones provided by OutputPage::addParserOutputContent() in PHP, that is modules, module styles, module scripts, module messages and JS config vars. (These are available via the action=parse&prop=modules API in core.)

Version: unspecified
Severity: major

Related Objects

Resolved GWicke
Resolved AlexMonk-WMF
Resolved marcoil
Resolved marcoil

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:35 AM
bzimport added a project: Parsoid.
bzimport set Reference to bz67540.




CSS modules for inclusion in the <head> would be a good start and will help reduce rendering diffs.

Change 160423 had a related patch set uploaded by Marcoil:
WIP: Bug 67540 - Load extension CSS modules

There's a WIP patch now at that adds the extensions' CSS modules to the style link.

If we also want to load further extension resources, like JS or messages, I suppose we'll have to add loading ResourceLoader's client first and let it load everything else. Is this the way we should go?

I think we'll want the lowlevel information about the modules available (via data-mw?), in addition to having One True Resource Loader URL which will load them all.

In particular two of OCG backends, the ZIM writer and the future parsoid-phantomjs-PDF backend, will want to download the set of resources used by a (potentially large) set of pages. We don't want to have to download the same resources in slight variations over and over again. If there are N different resources used on a wiki, there are 2^N different resource loader URLs which load various combinations of the N resources. We'd rather just load the N resources once, even if that means we have to do a little extra work ourselves to order and merge them.

(12:15:15 PM) gwicke: cscott: we should expose the list separately, but for browsers it's probably more efficient to still do some bundling
(12:15:29 PM) gwicke: good to unbundle the per-page modules from the global ones though
(12:15:35 PM) gwicke: right now those are all mixed afaik
(12:18:03 PM) cscott-free: gwicke: agreed. i'm thinking of exposing the list via data-mw (for example) while still providing the resource url in a <link> tag for browsers.

As discussed on IRC, we'll do a first patch only for CSS support, and then expand on it with the rest of extensions' resources. We can continue using this bug for this and for C. Scott's proposal of exposing the modules in metadata.

Btw, does anyone have examples of pages where an extension CSS causes Parsoid to render differently from PHP? I'd like to test the visual diffs with this patch. Thanks!

Change 160423 merged by jenkins-bot:
Bug 67540 - Load extension CSS modules

The merged patch takes care of adding the appropriate CSS modules to the load list, but only for extensions. Some parser functions (e.g. Babel) also have CSS modules, there's a patch for MW that would allow Parsoid to know about them here:

ssastry lowered the priority of this task from High to Medium.Feb 3 2015, 5:59 PM
ssastry moved this task from Needs Triage to In Progress on the Parsoid board.

BTW, i'm trying to move TMH from it's global context into a more isolated one, but i'm guessing that due to the state of this ticket, that will also mean that when that patch lands, it will actually break TMH videos when added by VE right ? Since the global handlers would be gone and the 'local' ones are not loaded yet ?

A little status update:

What currently works:

  • Parsoid correctly includes style modules from extensions present in the page that add them through ResourceLoader, so that when requesting the HTML directly from Parsoid it's presented correctly.

What doesn't:

  • Style modules added by parser functions, as the expandtemplates API doesn't expose them yet. A patch similar to @Jackmcbarn's, but without including non-RL modules would solve this.
  • All non-style modules exposed through ResourceLoader (Javascript, global vars, messages…). Parsoid's HTML output doesn't load ResourceLoader client infrastructure, so it's difficult to correctly load those modules.

I'm not sure if it's even a good idea to load non-style modules in Parsoid's HTML <head>, tbh, given that it's not really used independently by anything right now. I like @cscott's proposal of exposing the information in data-mw so that clients can use them as they see fit.

So my proposed next steps would be:

  • Expose all modules in action=expandtemplates, excluding non-RL modules.
  • Add the style ones to Parsoid's <head> as we do now with extension ones.
  • Expose all modules (styles and all others) through data-mw.

That way, for example, when VE receives just the body of a page for editing, it would look into the data-mw and load the modules it sees there, which would fix T69515 and related. Right now VE can't get that data directly as it's in Parsoid's <head>.

ssastry set Security to None.
ssastry moved this task from In Progress to Needs Triage on the Parsoid board.

Change 215874 had a related patch set uploaded (by Marcoil):
Expose ResourceLoader modules in action=expandtemplates

Change 215883 had a related patch set uploaded (by Marcoil):
Include RL style modules from parser functions in <head>

Change 215874 merged by jenkins-bot:
Expose RL modules and js config vars in action=expandtemplates

Change 215883 merged by jenkins-bot:
Include RL style modules from parser functions in <head>

There were two requests at wikimania for the rest of the stuff included in this bug. Namely:

  • Link to JS necessary to render the page contents. (Especially for maps modules, etc)
  • Some sort of machine-readable enumeration of the modules required (in case you wanted to do manual loading of CSS/JS/etc).
Arlolra claimed this task.
Arlolra added a subscriber: Arlolra.

Presumably this is resolved by T162399