If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED

Status

()

Core
HTML: Parser
P2
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

Other Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

8 years ago
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.
(Assignee)

Updated

8 years ago
Priority: -- → P2
(Assignee)

Comment 1

8 years ago
Created attachment 432821 [details] [diff] [review]
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.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Attachment #432821 - Flags: review?(bnewman)
(Assignee)

Updated

8 years ago
Summary: [HTML5] Isindex default prompt should be localizable → [HTML5][Patch] Isindex default prompt should be localizable
(Assignee)

Comment 2

8 years ago
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)
(Assignee)

Comment 3

8 years ago
Created attachment 433014 [details] [diff] [review]
Patch that returns a safe value on failure
Attachment #433014 - Flags: review?(bnewman)
(Assignee)

Comment 4

8 years ago
Created attachment 433015 [details] [diff] [review]
Patch that returns a safe value on failure, for real

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+
(Assignee)

Comment 6

8 years ago
Created attachment 433892 [details] [diff] [review]
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.
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+
(Assignee)

Comment 8

8 years ago
(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.
(Assignee)

Comment 9

8 years ago
Created attachment 438083 [details] [diff] [review]
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
Attachment #438083 - Flags: review?(bnewman)
Attachment #438083 - Flags: review?(bnewman) → review+
(Assignee)

Comment 10

8 years ago
http://hg.mozilla.org/mozilla-central/rev/e648fd6a9dee
http://hg.mozilla.org/mozilla-central/rev/4aa2109bb5c6
http://hg.mozilla.org/mozilla-central/rev/7db9990e3871
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

8 years ago
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.