Page MenuHomePhabricator

Mobile should not completely blank mediawiki.skinning.content.parsoid
Closed, ResolvedPublic

Description

VisualEditor is not the only consumer of the Parsoid styles. Flow needs them as well.

The solution is to move the general Parsoid styles from VisualEditorOverlay.less to Minerva's version of the 'mediawiki.skinning.content.parsoid' module.

You should consider whether you really want to maintain an entire fork of this module. A better solution may be to keep general-purpose styles in the 'mediawiki.skinning.content.parsoid' default module (which already exists in core), and move ones you consider too Vectorish into Vector's skinStyles, using the '+' option (see $wgResourceModuleSkinStyles).

Regardless of which approach you take, it needs to have feature parity (see e.g. T93856: an inline external link in square brackets without a title doesn't appear in Flow pages on mobile web).

Event Timeline

Mattflaschen-WMF raised the priority of this task from to Normal.
Mattflaschen-WMF updated the task description. (Show Details)
Mattflaschen-WMF updated the task description. (Show Details)
Mattflaschen-WMF set Security to None.
Mattflaschen-WMF added a subscriber: Jdlrobson.

Why does mediawiki.skinning.content.parsoid still exist? Wasn't that supposed to be a temporary hack until parsoid and parser DOM models were synced?

Why does mediawiki.skinning.content.parsoid still exist? Wasn't that supposed to be a temporary hack until parsoid and parser DOM models were synced?

The PHP parser and Parsoid (still) have entirely different DOMs. This includes not just the metadata, but even simple things like the output for:

[http://example.com]

PHP:

<a rel="nofollow" class="external autonumber" href="http://example.com">[1]</a>

Parsoid:

<a rel="mw:ExtLink" href="http://example.com" data-parsoid="{&quot;targetOff&quot;:19,&quot;contentOffsets&quot;:[19,19],&quot;dsr&quot;:[0,20,19,1]}"></a>

Note that the Parsoid one doesn't even have link text in the HTML. It uses CSS for that, per T93856: an inline external link in square brackets without a title doesn't appear in Flow pages on mobile web.

I haven't heard any recent discussion about having PHP and Parsoid output the same HTML. I have heard a little recent discussion about using Parsoid HTML for normal views (still a big project).

Let me know if I can help you update mobile's skinStyles here to fix the issues.

kaldari added a comment.EditedMar 26 2015, 9:29 PM

I don't mean they need to be the exact same HTML, I just mean that a year or so ago there was discussion about killing the two DOM differences that caused CSS duplication. For example, why does Cite output:

<sup class="reference">...</sup>

while parsoid outputs:

<span class="reference">...</span>

Surely these could be synced. The only other significant one is <img> vs. <figure>. Why can't we sync that one as well?

Anyway, I guess those should be filed as separate bugs (if they aren't already).

I don't mean they need to be the exact same HTML, I just mean that a year or so ago there was discussion about killing the two DOM differences that caused CSS duplication. For example, why does Cite output:

<sup class="reference">...</sup>

while parsoid outputs:

<span class="reference">...</span>

Surely these could be synced. The only other significant one is <img> vs. <figure>. Why can't we sync that one as well?

In theory they could be. But in practice, these differences remain, and Parsoid provides a stylesheet for its markup which currently needs to be applied (not just for styling, but for basic functionality like auto-numbered external links having a number).

These three (how auto-numbering works, Cite output, and img/figure) are probably not the only significant differences. (Note, figure is being introduced to core in https://gerrit.wikimedia.org/r/#/c/196532/ , but I don't know if it uses the exact same structure as Parsoid).

Sorry for the delayed response here .. but editing and roundtripping imposes additional requirements on Parsoid HTML that aren't there on PHP output. Without CSS-based auto-numbered elinks, for example, clients like VE would have to dirty a lot of the output in related areas of the document whenever one of them is edited.

The <figure> changes to core are a good thing since it will eliminate one of the big sources of diffs besides making that output more semantic.

There are more <cite> changes coming in Parsoid (first) which will then be adopted in core cite as well.

So, I expect some of these differences between Parsoid HTML and PHP parser HTML will get pared down, but for the last 3+ months, the focus has been entirely on supporting VE. We put read views on the backburner.

I know that doesn't help you immediately, but till the output diffs go away, parsoid output will need its own styles for some of the DOM elements. I expect this problem to get a little worse (with cite for ex where we will introduce some wiki-content-lang-specific stylesheets for reproducing the <ref> numbering schemes) before it gets better.

Why do we blank it out of interest?

If I understand eee42380952835112eebf785425d4f8d6d0b6ee2 correctly, it was basically done with the assumption that Parsoid styles are only applicable for VE. So it was just removed since Mobile had its own VE styles.

Parsoid styles are now used for more than just VE.

Arlolra moved this task from Backlog to Non-Parsoid Tasks on the Parsoid board.Feb 5 2016, 6:44 AM
Jdlrobson moved this task from Backlog to Tracking on the MobileFrontend board.Mar 16 2016, 10:38 PM
Jdlrobson lowered the priority of this task from Normal to Low.Jul 6 2016, 5:11 PM
Jdlrobson moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
TheDJ moved this task from Tracking to Bugs on the MobileFrontend board.Jan 3 2017, 12:46 PM
TheDJ moved this task from Bugs to Tracking on the MobileFrontend board.
TheDJ added a subscriber: TheDJ.

oops. accidental drag&drop on the board.

Jdlrobson closed this task as Resolved.May 21 2017, 11:34 AM
Jdlrobson claimed this task.

This doesn't appear to be blanked anymore.

ag parsoid extension.json

(no results)