Closed Bug 570898 Opened 10 years ago Closed 10 years ago

html5 parser ISINDEX_PROMPT initialized too early

Categories

(Core :: DOM: HTML Parser, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: benjamin, Assigned: hsivonen)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch Hardcode English string, rev. 1 (obsolete) — Splinter Review
I wrote the Java and C++ changes separately, and only tested the C++ version.
Attachment #450043 - Flags: review?(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.
Attachment #450043 - Flags: review?(hsivonen) → review-
Assignee: benjamin → hsivonen
Attachment #450043 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #450326 - Flags: review?(jonas)
Jonas, this blocks static-xpcom-registration, can you review this Monday?
http://hg.mozilla.org/mozilla-central/rev/075fe10df9c3
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.