500631 - [HTML5][Patch] Isindex default prompt should be localizable

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P2)

Description

[Henri Sivonen (:hsivonen)]

From bug 487949:
> >+jArray<PRUnichar,PRInt32>
> >+nsHtml5Portability::isIndexPrompt()
> >+{
> >+  // Yeah, this whole method is uncool
> >+  char* literal = "This is a searchable index. Insert your search keywords here: ";
> 
> This should be translatable.

Comment 1

[Henri Sivonen (:hsivonen)]

Attachment: Use the pre-existing localized string

As far as I can tell, the old code doesn't do anything special for right-to-left text, either.

Comment 2

[Henri Sivonen (:hsivonen)]

Comment on attachment 432821 [details] [diff] [review]
Use the pre-existing localized string

Hmm. Actually, the failure case probably needs more careful handling to avoid exposing uninitialized memory to Web content.

Comment 3

[Henri Sivonen (:hsivonen)]

Attachment: Patch that returns a safe value on failure

Comment 4

[Henri Sivonen (:hsivonen)]

Attachment: Patch that returns a safe value on failure, for real

Well that was embarrassing. Sorry about the bug spam.

Comment 5

[Ben Newman (:bnewman) (:benjamn)]

Comment on attachment 433015 [details] [diff] [review]
Patch that returns a safe value on failure, for real

Glad to see that 62 constant go away.  Looks good.

Comment 6

[Henri Sivonen (:hsivonen)]

Attachment: Put a space at the end of the isindex prompt

This patch puts a space at the end of the isindex prompt. It makes the HTML5 case look more proper by separating the text and the input field like they were separated in the non-HTML5 case. It doesn't appear to change the appearance of the non-HTML5 case.

Unfortunately, this bug will require changes to the html5lib test suite, so there will have to be a patch about that still. See also
http://www.w3.org/Bugs/Public/show_bug.cgi?id=9279
for the fun of exposing UI strings in the DOM and then comparing the DOM against a test suite.

Comment 7

[Ben Newman (:bnewman) (:benjamn)]

Comment on attachment 433892 [details] [diff] [review]
Put a space at the end of the isindex prompt

(In reply to comment #6)
> This patch puts a space at the end of the isindex prompt. It makes the HTML5
> case look more proper by separating the text and the input field like they were
> separated in the non-HTML5 case.

Can you think of any other cases where this kind of extra spacing might be necessary?

Comment 8

[Henri Sivonen (:hsivonen)]

(In reply to comment #7)
> Can you think of any other cases where this kind of extra spacing might be
> necessary?

I don't (other than localizations of this string, but I expect localizations to pick up the change made here using whatever mechanism they use for detecting changes in the UI strings). The only other parser macro is (for the time being still) <keygen>, and <keygen> doesn't need changes to the strings it exposes.

Comment 9

[Henri Sivonen (:hsivonen)]

Attachment: Refresh html5lib tree test data file #2 to upstream data

The spec was revised:
http://html5.org/tools/web-apps-tracker?from=4935&to=4936

I made the test case change in the upstream repo:
http://code.google.com/p/html5lib/source/diff?spec=svn46c9266982870bb8a07c44f0ae28b4f18fe6a24e&r=46c9266982870bb8a07c44f0ae28b4f18fe6a24e&format=side&path=/testdata/tree-construction/tests2.dat&old_path=/testdata/tree-construction/tests2.dat&old=36604e42b45c23f510278f39c1607e3f6e56824d

Comment 10

[Henri Sivonen (:hsivonen)]

http://hg.mozilla.org/mozilla-central/rev/e648fd6a9dee
http://hg.mozilla.org/mozilla-central/rev/4aa2109bb5c6
http://hg.mozilla.org/mozilla-central/rev/7db9990e3871

Comment 11

[Henri Sivonen (:hsivonen)]

Filed bug 559819 per IRC comments on the landing.