Page MenuHomePhabricator

Use CSS to style Parsoid's Cite extension output
Closed, ResolvedPublic

Description

From the discussion in wikitech-l at https://lists.wikimedia.org/pipermail/wikitech-l/2014-December/079844.html:

The Editing Department are replacing the current way of configuring the Cite extension in wikitext with some CSS rules (using ::after and counters), with some default bare HTML styles for older browsers like IE6.

  • Provide the styling in the Cite extension
  • Use the styling in Parsoid
  • Use the styling in VisualEditor
  • Use the styling in other consumers of Parsoid HTML

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
marcoil raised the priority of this task from to High.Jan 14 2015, 5:18 PM
marcoil claimed this task.
marcoil added a project: Parsoid.
marcoil added subscribers: marcoil, ssastry, GWicke and 2 others.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 14 2015, 5:18 PM
Nemo_bis added a subscriber: Nemo_bis.

If this request is going to affect users of the Cite extension in general, please add the MediaWiki-extensions-Cite component for a start.

marcoil set Security to None.

If this request is going to affect users of the Cite extension in general, please add the MediaWiki-extensions-Cite component for a start.

After we have used Parsoid to experiment with this, and we're satisfied with the results, then we'll adapt the PHP Cite extension. Although I suppose we'll track that work on a separate task, it's not a bad idea to add the project here, thanks for the suggestion.

Ltrlg added a subscriber: Ltrlg.Jan 14 2015, 7:08 PM
ssastry moved this task from Backlog to In Progress on the Parsoid board.Jan 14 2015, 8:19 PM
He7d3r added a subscriber: He7d3r.Jan 15 2015, 12:29 AM
fbstj awarded a token.Jan 15 2015, 8:28 AM

From email, referring to where to put the new CSS in core, @Jdforrester-WMF wrote:

Would it not make more sense to have this served from the Cite extension, rather than making the module be named Parsoid-specifically, so it can be shared?

I agree that it's a good idea, we just need to be careful to only include the new CSS in pages that use the new Cite HTML, or there can be unintended interactions with old HTML.

Next steps from the offline meeting with @ssastry, @GWicke, @Catrope and @Krinkle:

  • Provide a content diff to show differences between the old HTML Parsoid produces now and what it will produce with this patch, to help ensuring it won't break any client. (There are changes because the new HTML now includes support for showing a default style in older browsers)
  • To ensure minimal disruption with existing HTML produced by MW, we'll change the main reference CSS class from 'reference' to 'mw-reference'.
  • Move new Cite CSS from Parsoid to MW core, load it only when VE is activated as content.parsoid.less is right now. This will allow testing the new Parsoid Cite HTML without messing up caches or non-VE page views. Once we've been able to test more languages and we're happy with the results we can move it again to its final resting place, the Cite.php extension as part of its future update to CSS.
  • In PHP-land, we'll need to add a ResourceLoaderFileModule that looks at the content language and sends the correct language Cite CSS plus the common Cite CSS. (@Legoktm)

Thanks everyone for your help!

ssastry moved this task from In Progress to In Progress on the Parsoid board.Jan 31 2015, 1:18 AM
  • To ensure minimal disruption with existing HTML produced by MW, we'll change the main reference CSS class from 'reference' to 'mw-reference'.

This is required because once VE is exited, the CSS that is loaded by VE linger on and will thus impact default rendering even when VE is not active. This change proposed above is to deal with this scenrio.

A little update after the latest patch available at https://gerrit.wikimedia.org/r/#/c/170936/, which includes the new stuff done in master such as using data-mw.body.id, removing state from the Cite extension, etc.

First, here's some data on how the HTML changes between Cite.php, current Parsoid HTML and the proposed new Parsoid HTML to style it with CSS.

For a simple wikitext

A <ref>simple</ref>
<references />

PHP:

<p>A <sup id="cite_ref-1" class="reference"><a href="#cite_note-1"><span>[</span>1<span>]</span></a></sup></p>
<ol class="references">
<li id="cite_note-1"><span class="mw-cite-backlink"><b><a href="#cite_ref-1">^</a></b></span> <span class="reference-text">simple</span></li>
</ol>

Current parsoid:

<p>A <span about="#mwt2" class="reference" id="cite_ref-1" rel="dc:references" typeof="mw:Extension/ref"
           data-mw='{"name":"ref","body":{"id":"mw-reference-text-cite_note-1"},"attrs":{}}'>
    <a href="#cite_note-1">[1]</a>
</span></p>
<ol class="references" typeof="mw:Extension/references" about="#mwt4"
      data-mw='{"name":"references","attrs":{}}'>
    <li about="#cite_note-1" id="cite_note-1">
        <span rel="mw:referencedBy"><a href="#cite_ref-1">↑</a></span> <span id="mw-reference-text-cite_note-1" class="mw-reference-text" data-parsoid="{}">simple</span>
    </li>
</ol>

New proposed HTML:

<p>A <span about="#mwt2" class="mw-ref" id="cite_ref-1" rel="dc:references" typeof="mw:Extension/ref"
           data-mw='{"name":"ref","body":{"id":"mw-reference-text-cite_note-1"},"attrs":{}}'>
    <a href="#cite_note-1" style="counter-reset: mw-Ref 1;"><span class="mw-reflink-text">[1]</span></a>
</span></p>
<ol class="mw-references" typeof="mw:Extension/references" about="#mwt4"
      data-mw='{"name":"references","attrs":{}}'>
    <li about="#cite_note-1" id="cite_note-1">
        <a href="#cite_ref-1" rel="mw:referencedBy"><span class="mw-linkback-text">↑ </span></a> <span id="mw-reference-text-cite_note-1" class="mw-reference-text" data-parsoid="{}">simple</span>
    </li>
</ol>

The main differences are:

  • Changed the classes to avoid interference with the current CSS, and also make them all start with mw-ref.
  • Added counters to allow styling of reference links.
  • Added extra HTML for clients that don't support the necessary CSS to display the new styling, they'll render a style similar to the default one.

Visual rendering
Now, let's look a few screenshots. First off, the current PHP rendering:


Here's the current Parsoid rendering:

This is how the new Parsoid output renders, with the new CSS loaded:

And this is how the new Parsoid output looks in browsers that don't support CSS counters:

Interaction with Visual Editor
Here's how the current Parsoid output is rendered in VE:


And here's how it looks with the new Parsoid HTML but no new CSS loaded (mostly the same as the last case):

In contrast, here's how it looks if we load the new CSS (by hackily patching content.parsoid.less, a more permanent solution TBA):

The link repetition is due to VE removing the contents of the reference span and substituting with a new <a> that doesn't have the same class. I think this last change was introduced recently, as this same test about a month ago gave better results iirc.

Next steps

  • Develop a ResourceLoaderFileModule to correctly serve the new CSS from the Cite extension (in progress).
  • Update the patch tests so it merges cleanly into Parsoid once everything else is ready (in progress).
  • Investigate how to get the new HTML and CSS to properly display in VE. This would mean that the change from normal view to VE wouldn't change the perceived rendering of <ref>s.

For completeness, here's a screenshot with VE editing the new HTML and CSS, patched so that it copies the new HTML over:


As shown, the links are styled using CSS so that special groups are shown (like ɑ and β).

Change 207474 had a related patch set uploaded (by Marcoil):
WIP: T86782: Use CSS to style Parsoid's Cite HTML

https://gerrit.wikimedia.org/r/207474

Change 207483 had a related patch set uploaded (by Marcoil):
WIP: T86782: Load module ext.cite.style

https://gerrit.wikimedia.org/r/207483

Change 207487 had a related patch set uploaded (by Marcoil):
WIP: T86782: Horrible hack to make the new Cite CSS appear in VE

https://gerrit.wikimedia.org/r/207487

Change 170936 had a related patch set uploaded (by Marcoil):
WIP: T86782: Use CSS to style Cite references

https://gerrit.wikimedia.org/r/170936

Preliminary patches are available in gerrit. They are all considered WIP, but pushing them to get early feedback on the general approach.

To test these two, I recommend using Jack's parsoidview user js gadget, available at https://en.wikipedia.org/wiki/User:Jackmcbarn/parsoidview.js, changing it to also load ext.cite.style.

To test it with VisualEditor, there are two further patches:

Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptMay 18 2015, 3:15 PM

@marcoil, @ssastry, and I have sketched out the implementation, deployment and testing strategy going forward. From https://etherpad.wikimedia.org/p/CiteCSS.

Objectives

  • Migrate Cite configuration from wiki messages embedded in HTML to CSS-only customisations
  • (Depends-On:) Migrate Cite HTML in MediaWiki and Parsoid to be more stylable. (Current HTML is not flexible enough.)

Associated tasks

https://phabricator.wikimedia.org/T86782: Use CSS to style Parsoid's Cite extension output

Blocks:
https://phabricator.wikimedia.org/T73803: Cite extension doesn't use native digits
https://phabricator.wikimedia.org/T51538: Cite: Improve compatibility between Parsoid's port and the PHP extension
https://phabricator.wikimedia.org/T45235: Cite: Parsoid's port does not support all of the PHP extension's config options and i18n messages

Patches

https://gerrit.wikimedia.org/r/207474 (mediawiki/extensions/Cite) Use CSS to style Parsoid's Cite HTML
https://gerrit.wikimedia.org/r/170936 (mediawiki/services/parsoid) Use CSS to style Cite references
WIP because tests haven't been updated to new HTML yet

We should also do a HTML version number bump in Parsoid so it clearly communicates that HTML output has changed, in case clients want to do something with it. (Related task: https://phabricator.wikimedia.org/T98540 )

https://gerrit.wikimedia.org/r/207483 (mediawiki/extensions/VisualEditor) Load the new Cite style module from VE
https://gerrit.wikimedia.org/r/207487 (mediawiki/extensions/VisualEditor) A test patch for VE so that it creates the correct HTML to get the new Cite style
Needs to be squashed into https://gerrit.wikimedia.org/r/207483 as loading new styles with old VE causes problems when with new Parsoid HTML.
Needs changing from a hack to something else. The ve.dm.MWReferenceNode should extract the values of a citation from Parsoid (number, group, inner html of actual citation etc.) in a way that supports both current Parsoid and new Parsoid. Then the ve.ce.MWReferenceNode can consistently generate HTML that matches the new HTML for Parsoid and Cite – even if the data originally came from old Parsoid cache.

Deployment strategy

Deploy the HTML change in Parsoid/VisualEditor first. Announce how wikis can update
their local stylesheets to customise the new HTML to match their old preferences.

At this time Read mode/MediaWiki will continue to use the old HTML and wikitext messages for back-compat.
Once sufficient time has passed to allow wikis to migrate we can apply it to MediaWiki Cite extension.
At this point the css written by local wikis to "fix" VE view, will now apply to read mode.
The end result being that for readers there was no temporary regression in how things are rendered.

Deployment steps

Testing required:

  • Current VE is tested with new Parsoid HTML to make sure rendering is acceptable in edit mode (at least till such time there are changes in VE to load the new css resource module from core).
    • From testing: If VE loads 'ext.cite.style' right now, the references appear doubled, as the HTML VE creates for <ref>s isn't styled properly. So, patches 3 and 4 should be combined into (a working) one.
  • Verify that new CSS does not break old HTML
    • Use case: Before local wikis add their styling, VE/Cite ship their own. Once VE is unloaded, the read view switches to original rendering (while VE CSS is still in effect). As the new HTML and CSS all use different classes, this should be the case. I can't see any problems in my testing.
    • Use case: When local wikis start adding their styling, this will apply globally. Regardless of whether VE has been loaded or not.
    • Use case: After PHP Cite is updated, old HTML will still be served from parser and HTML caches for a month or so.
  • Till the stored HTML in RESTBase turns over, VE will get old cite HTML and new cite HTML. Need to confirm that editing experience is not broken because of this.

Other notes:

All other clients (Flow, CX) need a heads up as well.

  1. Merge/Deploy https://gerrit.wikimedia.org/r/207474 (mediawiki-Cite). This adds a new "ext.cite.style" module with default styles for the new HTML. Not used anywhere by default at this time.
  2. Merge/Deploy https://gerrit.wikimedia.org/r/170936 (parsoid). This will switch to using the new HTML and load "ext.cite.style".
    • Verify plain Parsoid renders look right (via parsoid webserver directly). At this point, VE will start being served new Cite HTML by Parsoid (for newly parsed revisions). VE has a custom ContentEditable rendering for citations that only includes a small part of Parsoid's <ref> HTML. So, as long as it does not load the conflicting "ext.cite.style" it will continue to look the same (current old way) in VE.
  3. Merge/Deploy https://gerrit.wikimedia.org/r/207483 (mediawiki-VisualEditor). This will change VE to consistently produce new-style CE HTML and loads "ext.cite.style".
    • At this point, the community is now able to customise the VE rendering of citations to match the old wikitext implementation.
    • Potential hazard: Load order of local wiki site css and ext.cite.style.
  4. ... (merge/deploy patch in mediawiki-Cite that uses new HTML and loads "ext.cite.style") ..
    • TODO: Write patch.

Differences in HTML

See https://phabricator.wikimedia.org/T86782#1233950

@mattflaschen, @santhosh FYI. Please comment here if this affects Flow / CX in any way.

Looks like we just need to merge https://gerrit.wikimedia.org/r/#/c/211766/ at the same time #3 is merged.

https://gerrit.wikimedia.org/r/#/c/207474/ for Cite implements the new ResourceLoader module ext.cite.style, which serves a common CSS file and, optionally, a content language-dependent one.

Note that there are usecases where we need to display the references in two languages in same wiki page. CX has to show source language content and target language content with two different references style. So if we load the style based on content language, we get style for only one language. How to solve this issue?

CX will need a change similar to patch by @Mattflaschen for Flow.(https://gerrit.wikimedia.org/r/#/c/211766/)

Note that there are usecases where we need to display the references in two languages in same wiki page. CX has to show source language content and target language content with two different references style. So if we load the style based on content language, we get style for only one language. How to solve this issue?

That's a very interesting use case… I suppose the situation right now is that all references look the same (the current Parsoid rendering), right?

On the CSS side, I think that using :lang() selectors (http://www.w3.org/International/questions/qa-css-lang) would actually be enough. The only "problem" I see right now is loading multiple style files as the current proposed patches use the wiki's content language and just load that one. We could have multiple ext.cite.style-<lang code> modules, but maybe it'd be better to do something dynamic instead of registering that many modules…

Certainly something to look into, but maybe as an improvement task instead of directly in this one?

CX will need a change similar to patch by @Mattflaschen for Flow.(https://gerrit.wikimedia.org/r/#/c/211766/)

Yes, I think that will be enough to get the main content language to display correctly, at least.

Thanks Santhosh for flagging that use case.

Note that there are usecases where we need to display the references in two languages in same wiki page. CX has to show source language content and target language content with two different references style. So if we load the style based on content language, we get style for only one language. How to solve this issue?

That's a very interesting use case… I suppose the situation right now is that all references look the same (the current Parsoid rendering), right?

On the CSS side, I think that using :lang() selectors (http://www.w3.org/International/questions/qa-css-lang) would actually be enough. The only "problem" I see right now is loading multiple style files as the current proposed patches use the wiki's content language and just load that one. We could have multiple ext.cite.style-<lang code> modules, but maybe it'd be better to do something dynamic instead of registering that many modules…

Certainly something to look into, but maybe as an improvement task instead of directly in this one?

Given that this is not a degradation of existing functionality, yes, I think this can be worked on in a followup.

Change 207487 abandoned by Marcoil:
WIP: T86782: Horrible hack to make the new Cite CSS appear in VE

Reason:
Merged into Ibf333a502d64d2ed6e029221458b7c606554e414.

https://gerrit.wikimedia.org/r/207487

Change 207474 had a related patch set uploaded (by Krinkle):
Use CSS to style Parsoid's Cite HTML

https://gerrit.wikimedia.org/r/207474

Change 207483 had a related patch set uploaded (by Krinkle):
Use Parsoid's new Cite HTML and CSS

https://gerrit.wikimedia.org/r/207483

Change 207474 had a related patch set uploaded (by Marcoil):
T86782: Use CSS to style Parsoid's Cite HTML

https://gerrit.wikimedia.org/r/207474

Change 207474 had a related patch set uploaded (by Krinkle):
Use CSS to style Parsoid's Cite HTML

https://gerrit.wikimedia.org/r/207474

Change 207474 merged by jenkins-bot:
Use CSS to style Parsoid's Cite HTML

https://gerrit.wikimedia.org/r/207474

For a quick test, I loaded ext.cite.style module to CX and I see the following result.

Before:


After:

Definitely this is not the expected result. What am I missing? The HTML of page is from https://simple.wikipedia.org/api/rest_v1/page/html/Oxygen

Definitely this is not the expected result.

You mean the "0" superscript, right? Or is there something else there?

You mean the "0" superscript, right? Or is there something else there?

Yes, duplicated arrows and 0 as superscript.

For a quick test, I loaded ext.cite.style module to CX and I see the following result.

...

Definitely this is not the expected result. What am I missing? The HTML of page is from https://simple.wikipedia.org/api/rest_v1/page/html/Oxygen

Parsoid HTML hasn't been updated yet. That cite module should be used once the Parsoid-side patch is merged and deployed ( https://gerrit.wikimedia.org/r/#/c/170936/ ).

Change 170936 had a related patch set uploaded (by Krinkle):
Use CSS to style Cite references

https://gerrit.wikimedia.org/r/170936

Change 170936 merged by jenkins-bot:
Use CSS to style Cite references

https://gerrit.wikimedia.org/r/170936

Change 207483 merged by jenkins-bot:
Use Parsoid's new Cite HTML and CSS

https://gerrit.wikimedia.org/r/207483

Jdforrester-WMF closed this task as Resolved.Jul 6 2015, 10:47 PM
Jdforrester-WMF added a project: Epic.
Jdforrester-WMF updated the task description. (Show Details)

So, now that this is live, the gadget for customizing the style (removing the brakets) of the references on ptwiki is not working anymore while editing with VisualEditor. Since there are plans to use Parsoid's HTML for normal page views (T55784), and there is no consensus on ptwiki for removing the (opt-out) gadget which removes the brackets, what is the recommended way to restore its behavior?
https://pt.wikipedia.org/wiki/MediaWiki:Gadget-hideRefBrakets.css?diff=38632412
https://pt.wikipedia.org/wiki/MediaWiki:Cite_reference_link?oldid=38632446&action=edit

@He7d3r Can we add an additional style to that sheet?

I did this, https://pt.wikipedia.org/wiki/Usu%C3%A1rio(a):Arlolra/common.css

span.mw-ref a::after {
  content: counter(mw-Ref,decimal);
}

And it seems to give the desired output.

That works. Thanks @Arlolra!

https://github.com/subbuss/parsoid_visual_diffs/blob/master/styles.yaml has enwiki-specific cite styles that leads to identical cite rendering for Parsoid HTML and PHP parser HTML output. These styles could be added to common.css on enwiki.

These styles could be added to common.css on enwiki.

By added, do you mean moved? I can't imagine why you'd want en.wiki-specific stuff in your repo.

These styles could be added to common.css on enwiki.

By added, do you mean moved? I can't imagine why you'd want en.wiki-specific stuff in your repo.

The visual diffs repo on github is the repo for comparing parsoid and php parser output. I created some of that CSS so that Parsoid HTML on enwiki could be styled using that CSS without site messages.

So yes, I meant: *copied* to enwiki's common.css and adapted so that Parsoid's HTML on enwiki looks identical to PHP parser HTML.