Error in errStrayStartTag key of dom/chrome/layout/htmlparser.properties

RESOLVED FIXED in mozilla10

Status

()

Core
DOM
--
trivial
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: rickiees, Assigned: rickiees)

Tracking

Trunk
mozilla10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 571178 [details] [diff] [review]
End vs. Start switch

errStrayStartTag key of htmlparser.properties is:

Stray end tag ā€œ%1$Sā€.

instead of:

Stray start tag ā€œ%1$Sā€.

So, "end" -> "start".

Patch provided.
Attachment #571178 - Flags: review?(hsivonen)
Comment on attachment 571178 [details] [diff] [review]
End vs. Start switch

Good catch. Thanks.
Attachment #571178 - Flags: review?(hsivonen) → review+
Ricardo, do you have access to land this patch yourself? If not, is it OK to use your bugzilla email in the patch author field?
(Assignee)

Comment 3

6 years ago
(In reply to Henri Sivonen (:hsivonen) from comment #2)
> Ricardo, do you have access to land this patch yourself? If not, is it OK to
> use your bugzilla email in the patch author field?


No, I don't have access, so I've set the checkin-needed keyword. Of course, it is OK to use my e-mail, so everybody can blame me if the Universe crash down after landing the patch. :-)
Keywords: checkin-needed
Thanks for the fix. Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/df8fb0f7eb07
Keywords: checkin-needed
Created attachment 571320 [details] [diff] [review]
Rename the key

It has been suggested to me that the string key should be renamed even though the erroneous message had been for less than 24 hours on m-c. Axel, should the key be renamed? Here's a patch for renaming the key in case it needs to be renamed.
Attachment #571320 - Flags: review?
Attachment #571320 - Flags: review? → review?(l10n)
Comment on attachment 571320 [details] [diff] [review]
Rename the key

Hrm. errNoElementToCloseButEndTagSeen is also wrong, having %1$S in there twice, I guess we want to fix that, too.

I'm unsure, does the suffix Bis carry any meaning?
(In reply to Axel Hecht [:Pike] from comment #6)
> Comment on attachment 571320 [details] [diff] [review] [diff] [details] [review]
> Rename the key
> 
> Hrm. errNoElementToCloseButEndTagSeen is also wrong, having %1$S in there
> twice, I guess we want to fix that, too.

It's intentional that the message contains the same tag name twice.

> I'm unsure, does the suffix Bis carry any meaning?

Not beyond the first meaning listed at http://en.wikipedia.org/wiki/Bis

I didn't want to use time to find out if digits are allowed in keys, so I didn't use errStartStrayTag2.
Comment on attachment 571320 [details] [diff] [review]
Rename the key

Review of attachment 571320 [details] [diff] [review]:
-----------------------------------------------------------------

Our printf code suprises me often, is there a test that verifies that the multiple %1$S works? Just haven't seen that done before, so I'm cautious.

Re the Bis, just use 2 as suffix, that is allowed and folks are used to it.
Attachment #571320 - Flags: review?(l10n) → review-
https://hg.mozilla.org/mozilla-central/rev/df8fb0f7eb07
leaving open for the l10n fix
Target Milestone: --- → mozilla10

Updated

6 years ago
Assignee: nobody → rpmdisguise-otros
I don't think that landing patches in the middle of the solution is helpful, tbh.

Now you're exposing the right string the wrong way.
Created attachment 571918 [details] [diff] [review]
Rename the key, v2
Attachment #571320 - Attachment is obsolete: true
Attachment #571918 - Flags: review?(l10n)
Comment on attachment 571918 [details] [diff] [review]
Rename the key, v2

Review of attachment 571918 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, thanks.
Attachment #571918 - Flags: review?(l10n) → review+
Thanks.
https://hg.mozilla.org/integration/mozilla-inbound/rev/936c243bf7fc
https://hg.mozilla.org/mozilla-central/rev/936c243bf7fc
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.