570898 - html5 parser ISINDEX_PROMPT initialized too early

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Benjamin Smedberg]

The HTML5 parser code initializes ISINDEX_PROMPT using stringbundles from the layout module initializer. This is bad for a lot of reasons, but two stand out:

* there's no guarantee that the chrome registry/necko/intl modules are registered yet
* the profile hasn't been started, so any langpacks and language prefs are not honored

This also happens to be causing problems with my static-registration patch. I tried to just look up the string dynamically, but the simple way of doing that doesn't work since the HTML5 parser doesn't run on the main thread. Since isindex is obsolete, the simplest solution here is to hardcode an English string.

Comment 1

[Benjamin Smedberg]

Attachment: Hardcode English string, rev. 1

I wrote the Java and C++ changes separately, and only tested the C++ version.

Comment 2

[Henri Sivonen (:hsivonen)]

Comment on attachment 450043 [details] [diff] [review]
Hardcode English string, rev. 1

(In reply to comment #0)
> This also happens to be causing problems with my static-registration patch. I
> tried to just look up the string dynamically, but the simple way of doing that
> doesn't work since the HTML5 parser doesn't run on the main thread. Since
> isindex is obsolete, the simplest solution here is to hardcode an English
> string.

There is a way the address the problems without hardcoding English: Making the tree builder queue a tree op that expands to the isindex prompt text node when the tree op is executed on the main thread. I'm not a fan of leaking the UI language to content, but if we choose to deliberately regress localizability and HTML5 compliance, I think we should do it for a reason that's not feasibly fixable by other means, such as limiting the site-exposed configuration entropy that can be used for fingerprinting.

I'm marking this patch r- for the following reasons:
 1) It regresses localizability where I think it's reasonably feasible not to regress localizability.
 2) It changes the actual prompt text that test cases depend on. The en-US version of the prompt needs to be exactly the text from the spec (which in turn is the string that Firefox and Chrome already use) in order to avoid breaking tests.
 3) The patch manually modifies generated code.

If the need to get rid of the string bundle dependency is urgent (I assume it is), I suggest backing out changeset https://hg.mozilla.org/mozilla-central/rev/4aa2109bb5c6 instead of landing this patch. (r=hsivonen for doing that.) I can write the patch to do the new tree op thing mentioned above, but I don't have the time to do it today.

Comment 3

[Henri Sivonen (:hsivonen)]

Attachment: Defer isindex prompt loading by using a tree op

Comment 4

[Benjamin Smedberg]

Jonas, this blocks static-xpcom-registration, can you review this Monday?

Comment 5

[Henri Sivonen (:hsivonen)]

http://hg.mozilla.org/mozilla-central/rev/075fe10df9c3