Last modified: 2013-04-19 04:44:25 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 T31542, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 29542 - mediawiki.jqueryMsg should wrap HTML tags around included templates/links
mediawiki.jqueryMsg should wrap HTML tags around included templates/links
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
JavaScript (Other open bugs)
1.18.x
All All
: Normal enhancement (vote)
: ---
Assigned To: Matthew Flaschen
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-23 06:33 UTC by Neil Kandalgaonkar
Modified: 2013-04-19 04:44 UTC (History)
5 users (show)

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


Attachments

Description Neil Kandalgaonkar 2011-06-23 06:33:09 UTC
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.)
Comment 1 Brion Vibber 2011-06-23 18:17:16 UTC
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);
});
Comment 2 Brion Vibber 2011-06-23 18:18:21 UTC
that'd be .replaceWith(link) :P
Comment 3 Neil Kandalgaonkar 2011-06-23 18:33:28 UTC
@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.
Comment 4 Neil Kandalgaonkar 2012-01-04 01:11:08 UTC
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.
Comment 5 Roan Kattouw 2012-01-04 16:37:51 UTC
(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.
Comment 6 Matthew Flaschen 2013-04-19 04:44:25 UTC
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).

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


Navigation
Links