Last Comment Bug 698935 - Error in errStrayStartTag key of dom/chrome/layout/htmlparser.properties
: Error in errStrayStartTag key of dom/chrome/layout/htmlparser.properties
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: mozilla10
Assigned To: [:rickiees] Ricardo Palomares
:
Mentors:
http://hg.mozilla.org/mozilla-central...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-01 16:20 PDT by [:rickiees] Ricardo Palomares
Modified: 2011-11-04 11:35 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
End vs. Start switch (1.44 KB, patch)
2011-11-01 16:20 PDT, [:rickiees] Ricardo Palomares
hsivonen: review+
Details | Diff | Review
Rename the key (2.47 KB, patch)
2011-11-02 06:22 PDT, Henri Sivonen (:hsivonen)
l10n: review-
Details | Diff | Review
Rename the key, v2 (2.46 KB, patch)
2011-11-04 03:26 PDT, Henri Sivonen (:hsivonen)
l10n: review+
Details | Diff | Review

Description [:rickiees] Ricardo Palomares 2011-11-01 16:20:26 PDT
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.
Comment 1 Henri Sivonen (:hsivonen) 2011-11-01 23:21:02 PDT
Comment on attachment 571178 [details] [diff] [review]
End vs. Start switch

Good catch. Thanks.
Comment 2 Henri Sivonen (:hsivonen) 2011-11-02 01:18:12 PDT
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?
Comment 3 [:rickiees] Ricardo Palomares 2011-11-02 01:59:56 PDT
(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. :-)
Comment 4 Henri Sivonen (:hsivonen) 2011-11-02 06:20:32 PDT
Thanks for the fix. Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/df8fb0f7eb07
Comment 5 Henri Sivonen (:hsivonen) 2011-11-02 06:22:48 PDT
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.
Comment 6 Axel Hecht [:Pike] 2011-11-02 07:30:56 PDT
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?
Comment 7 Henri Sivonen (:hsivonen) 2011-11-02 07:38:02 PDT
(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 8 Axel Hecht [:Pike] 2011-11-02 09:12:08 PDT
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.
Comment 9 Marco Bonardo [::mak] 2011-11-03 08:24:47 PDT
https://hg.mozilla.org/mozilla-central/rev/df8fb0f7eb07
leaving open for the l10n fix
Comment 10 Axel Hecht [:Pike] 2011-11-03 08:45:47 PDT
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.
Comment 11 Henri Sivonen (:hsivonen) 2011-11-04 03:26:03 PDT
Created attachment 571918 [details] [diff] [review]
Rename the key, v2
Comment 12 Axel Hecht [:Pike] 2011-11-04 06:05:09 PDT
Comment on attachment 571918 [details] [diff] [review]
Rename the key, v2

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

r=me, thanks.
Comment 13 Henri Sivonen (:hsivonen) 2011-11-04 06:38:16 PDT
Thanks.
https://hg.mozilla.org/integration/mozilla-inbound/rev/936c243bf7fc
Comment 14 Matt Brubeck (:mbrubeck) 2011-11-04 11:35:57 PDT
https://hg.mozilla.org/mozilla-central/rev/936c243bf7fc

Note You need to log in before you can comment on or make changes to this bug.