r80116 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r80115‎ | r80116 | r80117 >
Date:20:43, 12 January 2011
Author:tparscal
Status:resolved (Comments)
Tags:
Comment:
Fixed IE bug, you must use html:msg for the msg elements that jquery.localize uses to translate things otherwise IE chokes when you insert them cause it doesn't know about the msg element.
Modified paths:
  • /trunk/phase3/resources/jquery/jquery.localize.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/jquery/jquery.localize.js
@@ -6,9 +6,12 @@
77 * with localized text, elements with title-msg and alt-msg attributes will receive localized title
88 * and alt attributes.
99 *
 10+ * Note that "msg" elements must have html namespacing such as "<html:msg />" to be compatible with
 11+ * Internet Explorer.
 12+ *
1013 * Example:
1114 * <p class="somethingCool">
12 - * <msg key="my-message" />
 15+ * <html:msg key="my-message" />
1316 * <img src="something.jpg" title-msg="my-title-message" alt-msg="my-alt-message" />
1417 * </p>
1518 *
@@ -26,10 +29,11 @@
2730 * @param Object: options Map of options
2831 * * prefix: Message prefix to use when localizing elements and attributes
2932 */
 33+
3034 $.fn.localize = function( options ) {
3135 options = $.extend( { 'prefix': '' }, options );
3236 return $(this)
33 - .find( 'msg' )
 37+ .find( 'msg,html\\:msg' )
3438 .each( function() {
3539 $(this)
3640 .text( mediaWiki.msg( options.prefix + $(this).attr( 'key' ) ) )
@@ -50,4 +54,7 @@
5155 .removeAttr( 'alt-msg' );
5256 } )
5357 .end();
54 -};
\ No newline at end of file
 58+};
 59+
 60+// Let IE know about the msg tag before it's used...
 61+document.createElement( 'msg' );

Follow-up revisions

RevisionCommit summaryAuthorDate
r801751.17: MFT r79960, r80034, r80036, r80043, r80044, r80050, r80053, r80074, r80...catrope17:13, 13 January 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   00:46, 15 July 2011

The find('msg,html\\:msg') doesn't appear to work in IE 8 -- it finds neither 'msg' nor 'html:msg' bits for me when running the tests in r92113, r92126

#Comment by Catrope (talk | contribs)   11:15, 8 September 2011

Isn't this long since fixed? If so, could you check which rev fixed it and whether it needs to be backported to 1.18?

#Comment by Nikerabbit (talk | contribs)   11:21, 8 September 2011

Doesn't look like so, see r92329 and r92758.

#Comment by Catrope (talk | contribs)   12:56, 9 September 2011

Don't those revs fix this rev?

#Comment by Nikerabbit (talk | contribs)   18:25, 9 September 2011

To me the code looks still essentially the same after those two revisions.

#Comment by Krinkle (talk | contribs)   17:41, 13 September 2011

Marking this 'resolved' (due to lack for a better choise). Explanation / Time-line:

  • Originally jquery.localize wanted to use a custom tag ‎<msg>.
  • Problem: IE doesn't like unknown elements, needs createElement() hack to even make it behave properly. But that still leaves issues when trying to select it from JavaScript in some cases (ie. when not being in the "document"). This was attempted to be fixed by this revision (r80116), but as said. Still leaves unsolved issues.
  • Solution: Use "html:msg" instead of "msg", which IE seems to not have a struggle with selecting from.
  • Realisation: We now have two tags documented, we don't want to have people using them mixed and uncover bugs in one way and not in other ways. We wanna stick to 1 standard.
  • Idea: Drop support for "‎<msg>"-support and just stick to ‎<html:msg>, which as we learned works great in IE and other browsers are OK with it as well.
  • Action: r92329: Remove "‎<msg>"-support, only keep "‎<html:msg>" (we weren't using "‎<msg>" anywhere in existing code, including all of /trunk/extensions)
  • Realisation: OMG, something broke! IE6/7 cannot select ‎<html:msg>
  • Discovery: Apparently, in the past few months, it wasn't the addition of html:msg that magically fixed it for IE. It was the presence of both that made IE happy. IE6/7 returns ‎<html:msg> elements by querying elements by tag name "msg". Whereas all other browsers (including IE8+) only select them with "html:msg"
  • Ultimate solution: Only support ‎<html:msg> usage in to-be-localized html fragments, but keep "msg" in the selector because that's how IE6/7 can select those elements. This was done in r92758

Amen.

Status & tagging log