Closed Bug 500631 Opened 11 years ago Closed 10 years ago

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

Categories

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

Other Branch
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: hsivonen, Assigned: hsivonen)

Details

Attachments

(3 files, 2 obsolete files)

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.
Priority: -- → P2
As far as I can tell, the old code doesn't do anything special for right-to-left text, either.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Attachment #432821 - Flags: review?(bnewman)
Summary: [HTML5] Isindex default prompt should be localizable → [HTML5][Patch] Isindex default prompt should be localizable
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.
Attachment #432821 - Attachment is obsolete: true
Attachment #432821 - Flags: review?(bnewman)
Attachment #433014 - Flags: review?(bnewman)
Well that was embarrassing. Sorry about the bug spam.
Attachment #433014 - Attachment is obsolete: true
Attachment #433015 - Flags: review?(bnewman)
Attachment #433014 - Flags: review?(bnewman)
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.
Attachment #433015 - Flags: review?(bnewman) → review+
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.
Attachment #433892 - Flags: review?(bnewman)
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?
Attachment #433892 - Flags: review?(bnewman) → review+
(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.
Attachment #438083 - Flags: review?(bnewman) → review+
Filed bug 559819 per IRC comments on the landing.
You need to log in before you can comment on or make changes to this bug.