Page MenuHomePhabricator

mediawiki.jqueryMsg should wrap HTML tags around included templates/links
Closed, ResolvedPublic

Description

Author: neilk

Description:
In UploadWizard's frontend/jQuery oriented parsing library, currently a string message like:

'This is a <b>[$1 bold]</b> link'

will be incorrectly rendered, as something like this:

This is a <b></b><a href="yourlink">bold</a> link

The reason is that the system attempts to first parse out the wikitext elements, and then assumes all the other strings are jQuery-parseable, e.g.

[ $( "This is a <b>" ), link, $( "</b> link" ) ]

So the first <b> is simply closed, and the </b> is simply removed.

The idea here was to avoid parsing HTML in the wikitext grammar, but this shows they have to be parsed simultaneously.

(HTML which is fully outside of or fully inside of another element is parsed correctly.)


Version: 1.18.x
Severity: enhancement
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=44525

Details

Reference
bz29542

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:30 PM
bzimport set Reference to bz29542.

Might work to do something like using a placeholder <span/> and then .replace() the links into it:

var $foo = $( "<div>This is a <b><span id='qqq1'></span></b> link</div>" )
$.each(paramList, function(i, link) {

$foo.find('#qqq' + i).replace(link);

});

that'd be .replaceWith(link) :P

neilk wrote:

@brion: That approach might work with the MediaWiki parser but not this one. There are no string replacements or placeholders. It makes an tree of jQuery objects directly from PEG parsing.

So, the right answer is almost certainly to add primitive HTML parsing. Maybe just <b> and <i>. If someone is using other attributes or classes in their message they're doing it wrong, IMO.

neilk wrote:

Copying some design thoughts from email.


Consider:

There <b>{{PLURAL:$1|is|are}} $1 lights</b>!

in jQueryMsg, at one point, looks like this.

[ 
  'There are <b>', 
  [ 'PLURAL', 0, 'is', 'are' ], 
  ' ', 
  [ 'REPLACE', 0 ], 
  ' lights</b>!'
]

I did it this way because I like s-expressions & they are very easy to recursively walk when you have nested stuff. Anyway, the emitter is a fairly simple algorithm that walks the tree, jquery-ifying as it goes. However, that ultimately results in this:

There are <b></b>4 lights!

Because jQuerying the first string closes the </b>, and jQuerying the last one simply discards the unnecessary </b>. There might be a way to fix this if we .toString() and concatenate before jQuerying at each level.

Another approach might be for ResourceLoader to use Roan's suggested hack to preprocessing, and to send an HTML structure to the browser. Like:

<span>
  There 
  <b>
    <span class="mwplural" param="0">
      <span class="mwform">is</span>
      <span class="mwform">are</span>
    </span>
  <span class="mwparam" param="0"/>
  lights
  </b>
  !
</span>

That way, it could be "native" jQuery all the way through. I'm suggesting <span> because I'm afraid of custom HTML tags (e.g. <mwfunc>) not working in IE, and namespaced HTML (e.g. <mw:func>) doesn't always work either.

(In reply to comment #4)

Another approach might be for ResourceLoader to use Roan's suggested hack to
preprocessing, and to send an HTML structure to the browser. Like:

<span>
  There 
  <b>
    <span class="mwplural" param="0">
      <span class="mwform">is</span>
      <span class="mwform">are</span>
    </span>
  <span class="mwparam" param="0"/>
  lights
  </b>
  !
</span>

That way, it could be "native" jQuery all the way through. I'm suggesting
<span> because I'm afraid of custom HTML tags (e.g. <mwfunc>) not working in
IE, and namespaced HTML (e.g. <mw:func>) doesn't always work either.

Oooh, I like this idea. This also makes my original hack overriding the plural pfunc more attractive than writing my own PPFrame expansion (because the output target is HTML, not JSON). I'll re-discuss with Tim when I see him.

Fixed by 473a27e32ededfef32c1e573d488a5cbb71ca0fc (bug 44525). As Neil anticipated, I had to parse the HTML in the jqueryMsg parser.

I still prefer the idea of doing all the actual parsing on the server (bug 43499).