Page MenuHomePhabricator
Create Task
Maniphest T252038

<style> tag output for TemplateStyles by Parsoid has three CDATA wrappers
Closed, ResolvedPublic
Actions

Description

Legacy parser output (from mediawiki.org article)

<style data-mw-deduplicate="TemplateStyles:r3809392">.mw-parser-output .tpl-flex-gallery-perfteam{display:flex;flex-flow:row wrap;justify-content:space-around}.mw-parser-output .tpl-flex-gallery-perfteam a.image{display:inline-block;vertical-align:middle;overflow-y:hidden;height:150px}.mw-parser-output .tpl-flex-gallery-perfteam.ve-ce-branchNode{display:block}</style>

Parsoid output (from mediawiki.org Flow page):

<style data-mw-deduplicate="TemplateStyles:r3758719" typeof="mw:Extension/templatestyles mw:Transclusion" about="#mwt1" data-parsoid="{&quot;pi&quot;:[[{&quot;k&quot;:&quot;1&quot;}]],&quot;dsr&quot;:[0,25,null,null]}" data-mw="{&quot;parts&quot;:[{&quot;template&quot;:{&quot;target&quot;:{&quot;wt&quot;:&quot;Main page talk notes&quot;,&quot;href&quot;:&quot;./Template:Main_page_talk_notes&quot;},&quot;params&quot;:{&quot;1&quot;:{&quot;wt&quot;:&quot;&quot;}},&quot;i&quot;:0}}]}"><![CDATA[<![CDATA[<![CDATA[.mw-parser-output .mw-tpl-colorbox{box-sizing:border-box;margin:0.5em 0.5em 1em 0.5em;border-radius:0.2em;background:#fff;box-shadow:0 0 0.2em #999999}.mw-parser-output .mw-tpl-colorbox-title{background:#eaecf0;color:#000000;border-radius:0.2em 0.2em 0 0;padding:0.5em 1em 0.5em 1em}.mw-parser-output .mw-tpl-colorbox-title-icon{opacity:0.8}.mw-parser-output .mw-tpl-colorbox-title-corner{float:right;font-size:0.7em}.mw-parser-output .mw-tpl-colorbox-content{padding:0.5em 1em 0.5em 1em}]]]]]]><![CDATA[><![CDATA[>]]]]><![CDATA[>]]></style>

export.png (912×1 px, 190 KB)

Specifically:

<![CDATA[<![CDATA[<![CDATA[
…
]]]]]]><![CDATA[><![CDATA[>]]]]><![CDATA[>]]>

This seems excessive and maybe by accident? Does it need CDATA at all, given there is no < in the content (which is the criteria Html.php uses).

Details

SubjectRepoBranchLines +/-
Support TemplateStyles in Structured Discussionsmediawiki/extensions/Flowmaster+38 -2
Customize query in gerrit

Related Objects
Search...

Event Timeline

Krinkle created this task.May 6 2020, 4:10 PM
Restricted Application added subscribers: Liuxinyu970226, Aklapper. · View Herald TranscriptMay 6 2020, 4:10 PM
Krinkle added a project: Performance-Team (Radar).May 6 2020, 4:10 PM
Krinkle moved this task from Limbo to Watching on the Performance-Team (Radar) board.
Arlolra subscribed.May 6 2020, 5:18 PM

Parsoid output (from mediawiki.org Flow page):

Though https://www.mediawiki.org/api/rest_v1/page/html/Wikimedia_Performance_Team gives,

<style data-mw-deduplicate="TemplateStyles:r3809392" typeof="mw:Extension/templatestyles" about="#mwt3" data-mw='{"name":"templatestyles","attrs":{"src":"Wikimedia Performance Team/styles.css"}}' id="mwBw">.mw-parser-output .tpl-flex-gallery-perfteam{display:flex;flex-flow:row wrap;justify-content:space-around}.mw-parser-output .tpl-flex-gallery-perfteam a.image{display:inline-block;vertical-align:middle;overflow-y:hidden;height:150px}.mw-parser-output .tpl-flex-gallery-perfteam.ve-ce-branchNode{display:block}</style>
Krinkle added a project: StructuredDiscussions.May 6 2020, 7:53 PM
Restricted Application added a project: Growth-Team. · View Herald TranscriptMay 6 2020, 7:53 PM
• ssastry removed a project: Parsing-Team--ARCHIVED.May 6 2020, 7:55 PM
• ssastry moved this task from Needs Triage to Non-Parsing-Team Tasks on the Parsoid board.May 6 2020, 8:19 PM
TheDJ subscribed.May 25 2020, 12:57 PM
Tacsipacsi subscribed.Aug 21 2020, 11:47 PM

Any progress? It quite destroys templates like d:Template:Statement+, see for example Topic:Vsh0uyvwlz91g81b.

Krinkle added a comment.Aug 22 2020, 12:18 AM

@Tacsipacsi This task is about an invisibible implementation detail of the HTML payload which is inefficient for bandwidth. It sounds like you're experiencing a bug/defect which is likely unrelated and should be reported separately.

Tacsipacsi added a comment.Aug 22 2020, 12:25 AM

I don’t know exactly how it works, but when I changed the stylesheet source code to remove the CDATA things, it fixed itself. This doesn’t work:

<style data-mw-deduplicate="TemplateStyles:r1237934680" typeof="mw:Extension/templatestyles" about="#mwt10" data-mw="{&quot;name&quot;:&quot;templatestyles&quot;,&quot;attrs&quot;:{&quot;src&quot;:&quot;Statement+/styles.css&quot;}}"><![CDATA[<![CDATA[<![CDATA[.mw-parser-output .statementplus{width:80%}.mw-parser-output .statementplus>table{width:100%;border:1px solid #c8ccd1;border-collapse:collapse;border-spacing:0;background-color:white}.mw-parser-output .statementplus-showtables>table{border-color:black;border-width:2px}.mw-parser-output .statementplus-property{vertical-align:top;padding-top:.8em;background-color:#eaecf0;width:20%}.mw-parser-output .statementplus[dir="ltr"] .statementplus-property{padding-left:1em}.mw-parser-output .statementplus[dir="rtl"] .statementplus-property{padding-right:1em}.mw-parser-output .statementplus-doc{vertical-align:top;padding:.8em 1em 0}.mw-parser-output .statementplus-addtoolbar{background-color:#eaecf0}.mw-parser-output .statementplus-addtoolbar-tool{width:20%;vertical-align:top;padding:.3em 0 .2em;color:silver}.mw-parser-output .statementplus[dir="ltr"] .statementplus-addtoolbar-tool{padding-left:1em}.mw-parser-output .statementplus[dir="rtl"] .statementplus-addtoolbar-tool{padding-right:1em}.mw-parser-output .statementplus-addtoolbar-tool::before{display:inline;content:" + "}.mw-parser-output .statementplus-value{width:100%;border-collapse:collapse;border-spacing:0;border-top:1px solid #eaecf0}.mw-parser-output .statementplus-value:first-child{border-top:none}.mw-parser-output .statementplus-showtables .statementplus-value{border:2px solid blue}.mw-parser-output .statementplus-rank{vertical-align:top;width:2em;padding-top:.45em}.mw-parser-output .statementplus-value[dir="ltr"] .statementplus-rank{padding-left:2px}.mw-parser-output .statementplus-value[dir="rtl"] .statementplus-rank{padding-right:2px}.mw-parser-output .statementplus-mainsnak{vertical-align:top;padding-top:.7em}.mw-parser-output .statementplus-editlink{vertical-align:top;padding-top:.7em;width:25%;color:silver}.mw-parser-output .statementplus-value[dir="ltr"] .statementplus-editlink{padding-left:.75em}.mw-parser-output .statementplus-value[dir="rtl"] .statementplus-editlink{padding-right:.75em}.mw-parser-output .statementplus-editlink>span{background:url("https://upload.wikimedia.org/wikipedia/commons/thumb/6/63/Arbcom_ru_editing.svg/14px-Arbcom_ru_editing.svg.png")no-repeat}.mw-parser-output .statementplus-value[dir="ltr"] .statementplus-editlink>span{padding-left:20px;background-position:left}.mw-parser-output .statementplus-value[dir="rtl"] .statementplus-editlink>span{padding-right:20px;background-position:right}.mw-parser-output .statementplus-references-toggle{color:#0c45ab}.mw-parser-output .statementplus-references-toggle::before{display:inline;content:"  ▼ "}.mw-parser-output .statementplus-value[dir="ltr"] .mw-collapsed>.statementplus-references-toggle::before{content:"  ► "}.mw-parser-output .statementplus-value[dir="rtl"] .mw-collapsed>.statementplus-references-toggle::before{content:"  ◀ "}.mw-parser-output .statementplus-reference{margin-top:.5em;background-color:#f7f8f9}.mw-parser-output .statementplus-reference:first-child{margin-top:0}.mw-parser-output .statementplus-addreference{box-sizing:border-box;width:25%;padding:.4em 0 1.2em;color:silver}.mw-parser-output .statementplus-value[dir="ltr"] .statementplus-addreference{float:right;padding-left:1.2em}.mw-parser-output .statementplus-value[dir="rtl"] .statementplus-addreference{float:left;padding-right:1.2em}.mw-parser-output .statementplus-addreference::before{display:inline;content:" + ";font-weight:bold}.mw-parser-output .statementplus-block{border-collapse:collapse;border-spacing:0;width:100%}.mw-parser-output .statementplus-showtables .statementplus-block{border:2px solid orange}.mw-parser-output .statementplus-block>tbody>tr>td{vertical-align:top}.mw-parser-output .statementplus-block[dir="ltr"]>tbody>tr>td{padding-left:.5em}.mw-parser-output .statementplus-block[dir="rtl"]>tbody>tr>td{padding-right:.5em}.mw-parser-output .statementplus-block>tbody>tr>td:first-child{width:12em}]]]]]]><![CDATA[><![CDATA[>]]]]><![CDATA[>]]></style>

while this one works:

<style data-mw-deduplicate="TemplateStyles:r1237934680" typeof="mw:Extension/templatestyles" about="#mwt10" data-mw="{&quot;name&quot;:&quot;templatestyles&quot;,&quot;attrs&quot;:{&quot;src&quot;:&quot;Statement+/styles.css&quot;}}">.mw-parser-output .statementplus{width:80%}.mw-parser-output .statementplus>table{width:100%;border:1px solid #c8ccd1;border-collapse:collapse;border-spacing:0;background-color:white}.mw-parser-output .statementplus-showtables>table{border-color:black;border-width:2px}.mw-parser-output .statementplus-property{vertical-align:top;padding-top:.8em;background-color:#eaecf0;width:20%}.mw-parser-output .statementplus[dir="ltr"] .statementplus-property{padding-left:1em}.mw-parser-output .statementplus[dir="rtl"] .statementplus-property{padding-right:1em}.mw-parser-output .statementplus-doc{vertical-align:top;padding:.8em 1em 0}.mw-parser-output .statementplus-addtoolbar{background-color:#eaecf0}.mw-parser-output .statementplus-addtoolbar-tool{width:20%;vertical-align:top;padding:.3em 0 .2em;color:silver}.mw-parser-output .statementplus[dir="ltr"] .statementplus-addtoolbar-tool{padding-left:1em}.mw-parser-output .statementplus[dir="rtl"] .statementplus-addtoolbar-tool{padding-right:1em}.mw-parser-output .statementplus-addtoolbar-tool::before{display:inline;content:" + "}.mw-parser-output .statementplus-value{width:100%;border-collapse:collapse;border-spacing:0;border-top:1px solid #eaecf0}.mw-parser-output .statementplus-value:first-child{border-top:none}.mw-parser-output .statementplus-showtables .statementplus-value{border:2px solid blue}.mw-parser-output .statementplus-rank{vertical-align:top;width:2em;padding-top:.45em}.mw-parser-output .statementplus-value[dir="ltr"] .statementplus-rank{padding-left:2px}.mw-parser-output .statementplus-value[dir="rtl"] .statementplus-rank{padding-right:2px}.mw-parser-output .statementplus-mainsnak{vertical-align:top;padding-top:.7em}.mw-parser-output .statementplus-editlink{vertical-align:top;padding-top:.7em;width:25%;color:silver}.mw-parser-output .statementplus-value[dir="ltr"] .statementplus-editlink{padding-left:.75em}.mw-parser-output .statementplus-value[dir="rtl"] .statementplus-editlink{padding-right:.75em}.mw-parser-output .statementplus-editlink>span{background:url("https://upload.wikimedia.org/wikipedia/commons/thumb/6/63/Arbcom_ru_editing.svg/14px-Arbcom_ru_editing.svg.png")no-repeat}.mw-parser-output .statementplus-value[dir="ltr"] .statementplus-editlink>span{padding-left:20px;background-position:left}.mw-parser-output .statementplus-value[dir="rtl"] .statementplus-editlink>span{padding-right:20px;background-position:right}.mw-parser-output .statementplus-references-toggle{color:#0c45ab}.mw-parser-output .statementplus-references-toggle::before{display:inline;content:"  ▼ "}.mw-parser-output .statementplus-value[dir="ltr"] .mw-collapsed>.statementplus-references-toggle::before{content:"  ► "}.mw-parser-output .statementplus-value[dir="rtl"] .mw-collapsed>.statementplus-references-toggle::before{content:"  ◀ "}.mw-parser-output .statementplus-reference{margin-top:.5em;background-color:#f7f8f9}.mw-parser-output .statementplus-reference:first-child{margin-top:0}.mw-parser-output .statementplus-addreference{box-sizing:border-box;width:25%;padding:.4em 0 1.2em;color:silver}.mw-parser-output .statementplus-value[dir="ltr"] .statementplus-addreference{float:right;padding-left:1.2em}.mw-parser-output .statementplus-value[dir="rtl"] .statementplus-addreference{float:left;padding-right:1.2em}.mw-parser-output .statementplus-addreference::before{display:inline;content:" + ";font-weight:bold}.mw-parser-output .statementplus-block{border-collapse:collapse;border-spacing:0;width:100%}.mw-parser-output .statementplus-showtables .statementplus-block{border:2px solid orange}.mw-parser-output .statementplus-block>tbody>tr>td{vertical-align:top}.mw-parser-output .statementplus-block[dir="ltr"]>tbody>tr>td{padding-left:.5em}.mw-parser-output .statementplus-block[dir="rtl"]>tbody>tr>td{padding-right:.5em}.mw-parser-output .statementplus-block>tbody>tr>td:first-child{width:12em}</style>
Tgr subscribed.Aug 22 2020, 2:36 PM

This looks wrong on so many levels. CDATA sections should not be used in HTML5 (they are meant specifically for including SVG or MathML content); using them elsewhere is technically a parser error and the spec instructs browsers to discard the contents. Even if CDATA were allowed, it cannot be nested. And it's not even properly nested.

MBinder_WMF edited projects, added Parsoid (Tracking); removed Parsoid.Dec 10 2020, 8:12 PM
stjn subscribed.Apr 8 2021, 2:06 PM
In T252038#6403572, @Tacsipacsi wrote:

I don’t know exactly how it works, but when I changed the stylesheet source code to remove the CDATA things, it fixed itself. This doesn’t work:

Bugzilla task on this filed 16 years ago (‘A CDATA section that appears inside a STYLE tag will cause the contents of the STYLE tag to be ignored.’):
https://bugzilla.mozilla.org/show_bug.cgi?id=285024

This should be given more priority, as it is unlikely that any browser (same in Chrome) will change its behaviour in regards to this, so using TS in Flow is practically impossible.

MBinder_WMF added a project: Growth-Team-Filtering.Apr 15 2021, 6:47 PM
ExE-Boss subscribed.Jun 24 2021, 11:19 AM
ExE-Boss added a comment.EditedJun 24 2021, 11:22 AM

The simple fix is to escape the <![CDATA[...]]> tags in a CSS comment, e.g. change:

<style data-mw-deduplicate="TemplateStyles:r4662631" typeof="mw:Extension/templatestyles mw:Transclusion" about="#mwt1" data-parsoid='{"pi":[[{"k":"1"}]],"dsr":[28,40,null,null]}' data-mw='{"parts":[{"template":{"target":{"wt":"T","href":"./Template:T"},"params":{"1":{"wt":"285463"}},"i":0}}]}'><![CDATA[<![CDATA[<![CDATA[.mw-parser-output .phui-tag{background-color:#e7e7e7;border:1px solid #e7e7e7;border-radius:3px;font:13px"Segoe UI","Segoe UI Web Regular","Segoe UI Symbol","Lato","Helvetica Neue",Helvetica,Arial,sans-serif;line-height:1.51em;padding:0 4px}.mw-parser-output .phui-tag>a.external{background:none!important;padding:0!important}.mw-parser-output .phui-tag>a,.mw-parser-output .phui-tag>a:visited{color:black}.mw-parser-output .phui-tag>a{font-weight:bold}]]]]]]><![CDATA[><![CDATA[>]]]]><![CDATA[>]]></style>

to:

<style data-mw-deduplicate="TemplateStyles:r4662631" typeof="mw:Extension/templatestyles mw:Transclusion" about="#mwt1" data-parsoid='{"pi":[[{"k":"1"}]],"dsr":[28,40,null,null]}' data-mw='{"parts":[{"template":{"target":{"wt":"T","href":"./Template:T"},"params":{"1":{"wt":"285463"}},"i":0}}]}'>/*<![CDATA[*//*<![CDATA[*//*<![CDATA[*/.mw-parser-output .phui-tag{background-color:#e7e7e7;border:1px solid #e7e7e7;border-radius:3px;font:13px"Segoe UI","Segoe UI Web Regular","Segoe UI Symbol","Lato","Helvetica Neue",Helvetica,Arial,sans-serif;line-height:1.51em;padding:0 4px}.mw-parser-output .phui-tag>a.external{background:none!important;padding:0!important}.mw-parser-output .phui-tag>a,.mw-parser-output .phui-tag>a:visited{color:black}.mw-parser-output .phui-tag>a{font-weight:bold}/*]]>*//*]]>*//*]]>*/</style>

Which works in all browsers, see also:

ExE-Boss added a parent task: T272941: Make TemplateStyles extension compatible with Parsoid.Jun 24 2021, 5:31 PM
TheDJ added a comment.Oct 11 2021, 7:42 PM

I ran into this again and was annoyed :)

This problem occurs because Flow reparses the Parsoid output with php's libxml and DOMDocument and then saves the document using saveXml instead of saveHTML. This happens in includes/Conversion/Utils.php in the getOuterHtml function. It is accompanied with the note: "Don't use saveHTML(), it has bugs (T217766); instead use XML serialization with a workaround for empty non-void nodes"
When it does this several times, you get this nested multiple level escaping of CDATA.

Likely this part of its code should be rewritten to use RemexHtml, but as it is so well delineated, we can probably work around it with a simple str_replace instead.

gerritbot added a comment.Oct 11 2021, 8:17 PM

Change 730042 had a related patch set uploaded (by TheDJ; author: TheDJ):

[mediawiki/extensions/Flow@master] Support TemplateStyles in Structured Discussions

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

gerritbot added a project: Patch-For-Review.Oct 11 2021, 8:17 PM
gerritbot added a comment.Oct 13 2021, 4:59 AM

Change 730042 merged by jenkins-bot:

[mediawiki/extensions/Flow@master] Support TemplateStyles in Structured Discussions

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

ReleaseTaggerBot added a project: MW-1.38-notes (1.38.0-wmf.5; 2021-10-19).Oct 13 2021, 5:00 AM
Maintenance_bot removed a project: Patch-For-Review.Oct 13 2021, 5:10 AM
TheDJ closed this task as Resolved.Oct 13 2021, 9:56 AM
TheDJ claimed this task.
TheDJ added a comment.Oct 20 2021, 6:43 PM

Because flow stores it's html output, this of course doesn't fix existing content, but if you take a previously broken piece of content like https://www.wikidata.org/wiki/Topic:Vsh0uyvwlz91g81b, choose edit, source editor and then resave, then the template styles are now present.

TheDJ removed projects: Parsoid (Tracking), MediaWiki-Parser.Oct 20 2021, 6:47 PM
TheDJ mentioned this in T293925: StructuredDiscussions has broken DOM transformations.Oct 20 2021, 6:56 PM
Tgr added a comment.Oct 20 2021, 10:55 PM

Eventually existing content will be fixed when T209120: Upgrade Parsoid HTML stored in the StructuredDiscussions tables gets done.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits