Last modified: 2014-07-16 00:20:41 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 T59429, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 57429 - VisualEditor: MWExtensionNode preview (<math>, <syntaxhighlight>, …) double-escapes HTML (angle brackets, ampersands)
VisualEditor: MWExtensionNode preview (<math>, <syntaxhighlight>, …) double-e...
Status: RESOLVED FIXED
Product: VisualEditor
Classification: Unclassified
Editing Tools (Other open bugs)
unspecified
All All
: Low minor
: VE-deploy-2014-07-17
Assigned To: Bartosz Dziewoński
:
: 60113 60332 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-22 17:27 UTC by Liangent
Modified: 2014-07-16 00:20 UTC (History)
6 users (show)

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


Attachments

Description Liangent 2013-11-22 17:27:58 UTC
Repro:

1. Create a page with text below:

<math>
\begin{matrix}
x & y \\
z & v
\end{matrix}
</math>

2. Default rendering, in PHP parser and Parsoid (VE initial state):

x y
z v

3. VE edit this matrix, change v to uv.

4. Edited block becomes:

x amp; y
z amp; uv

(After saving it works fine)
Comment 1 James Forrester 2014-07-03 23:44:59 UTC
*** Bug 60113 has been marked as a duplicate of this bug. ***
Comment 2 Bartosz Dziewoński 2014-07-04 15:13:15 UTC
This problem is actually common for everything that uses MWExtensionNode. The problem is that ve.ce.MWExtensionNode.prototype.generateContents makes some bold assumptions about wikitext syntax that are not true.

The fix from bug 54577 didn't actually fix much, it just changed what the problem is.
Comment 3 Bartosz Dziewoński 2014-07-04 15:13:46 UTC
*** Bug 60332 has been marked as a duplicate of this bug. ***
Comment 4 Bartosz Dziewoński 2014-07-04 15:24:42 UTC
Basically, ve.ce.MWExtensionNode.prototype.generateContents builds an XML element object for given tag name (for example 'math'), given attributes (none in this case) and given contents (for example 'a < b'), and then serializes it to a string to ensure that everything is correctly escaped.

The problem is that MediaWiki XML-like tags don't expect their contents to be escaped. Serializing such a node generates "<math>a &lt; b</math>", while wikitext expects "<math>a < b</math>".

(An interesting corollary is that a MediaWiki XML-like tag 'foo' may not contain the string '</foo>' in any form nor encoded in any way, unless the code of the extension handling the tag specifically parses nested <nowiki/> tags or something. You can cheat – {{#tag:foo|</foo>}} – but then you still can't include the string '<foo></foo>' because it gets double-parsed.)

(Fun fact: the above behavior is the reason why <source/> was renamed to <syntaxhighlight/> some years ago.)

We should not escape the contents at all and instead do something clever if the input text contains '</foo>'. VE and wikitext are both designed to never disallow the user from doing stupid things, so I'm not sure what can be done if we don't just prevent the user from saving that. Maybe selectively HTML-escape /<\/\s*foo\s*>/ when closing the inspector?
Comment 5 Bartosz Dziewoński 2014-07-04 15:27:37 UTC
Just for "fun", some informative examples that show what happens when you try too hard with {{#tag:}}. Two of these actually cause UNIQ markers to appear in output, which is rather bad in general. The behavior may differ depending on the tag used.


<syntaxhighlight lang=php>$a;</syntaxhighlight>

<syntaxhighlight lang=php></syntaxhighlight></syntaxhighlight>

<syntaxhighlight lang=php>&lt;/syntaxhighlight></syntaxhighlight>

{{#tag:syntaxhighlight|</syntaxhighlight>|lang=php}}

<syntaxhighlight lang=php><syntaxhighlight lang=php>$a;</syntaxhighlight></syntaxhighlight>

{{#tag:syntaxhighlight|<syntaxhighlight lang=php>$a;</syntaxhighlight>|lang=php}}

{{#tag:syntaxhighlight|&lt;syntaxhighlight lang=php>$a;</syntaxhighlight>|lang=php}}

{{#tag:syntaxhighlight|<nowiki><syntaxhighlight lang=php>$a;</syntaxhighlight></nowiki>|lang=php}}
Comment 6 Gerrit Notification Bot 2014-07-04 16:14:00 UTC
Change 144157 had a related patch set uploaded by Bartosz Dziewoński:
ve.ui.MWExtensionInspector: Prevent from setting impossible content

https://gerrit.wikimedia.org/r/144157
Comment 7 Gerrit Notification Bot 2014-07-04 16:14:05 UTC
Change 144158 had a related patch set uploaded by Bartosz Dziewoński:
ve.ce.MWExtensionNode: Don't escape content of wikitext tags on preview

https://gerrit.wikimedia.org/r/144158
Comment 8 Gerrit Notification Bot 2014-07-15 22:09:08 UTC
Change 144157 merged by jenkins-bot:
ve.ui.MWExtensionInspector: Prevent from setting impossible content

https://gerrit.wikimedia.org/r/144157
Comment 9 Gerrit Notification Bot 2014-07-15 22:09:12 UTC
Change 144158 merged by jenkins-bot:
ve.ce.MWExtensionNode: Don't escape content of wikitext tags on preview

https://gerrit.wikimedia.org/r/144158
Comment 10 Bartosz Dziewoński 2014-07-15 22:36:06 UTC
Should be fixed.

Like in wikitext, it will now be impossible to input '</tag>' in the inspector in VisualEditor: it will be converted to '&lt;/tag>' when the inspector is closed.
Comment 11 James Forrester 2014-07-16 00:20:41 UTC
Thanks Bartosz!

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


Navigation
Links