438562 - Bad html in localizable string

Status: ASSIGNED

(Thunderbird :: Security, defect)

Description

[Hendrik Maryns]

Attachment: fix the html

The entity CantDecryptBody uses bad html.  See fix in patch.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Is <br /> valid in text/html documents?  We're not in XHTML here.  

Also, what problem are you trying to solve?

Comment 2

[Phil Ringnalda (:philor)]

<br / is a NET-enabling start tag; in HTML br is an empty element; <br /> is therefore exactly equal to <br>>, but it's used in Appendix C uses of XHTML-as-HTML because the (incorrect) error handling of browsers treats it as a br element with a minimized attribute named /, which is an invalid attribute name so it's ignored. This patch doesn't solve any problem, it's fetishism. The existing string is perfectly correct HTML, the proposed version is incorrect and depends on unspecified error handling.

The only thing that would be a "fix" here would be to recast the string (probably into several separate strings) so that no markup is required within the localizable string: *that* would be worth doing, and would serve a purpose.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 324609 [details] [diff] [review]
fix the html

r- on the grounds described by Phil

Comment 4

[Hendrik Maryns]

Comment on attachment 324609 [details] [diff] [review]
fix the html

Thanks for clearing me up on that.  Actually, I like being called a fetishist, it’s how I call myself on my website when talking about Unicode :-) 

Anyway, I’ll take your comments and try to split the string up in parts.

Comment 5

[Hendrik Maryns]

Attachment: split up string

First try: I split up the string and put all the html tags in the corresponding js file.  How’s this?

I am unsure about the names, I should probably use something more descriptive, but please confirm that the idea is correct!

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 326112 [details] [diff] [review]
split up string

I'm gonna hand this back to Phil, as my review queue is pretty deep, so it's likely to be a while before I'd have time to get to this.

Comment 7

[Phil Ringnalda (:philor)]

Comment on attachment 326112 [details] [diff] [review]
split up string

Nice tangle: the only reason you can get away with changing the strings in /suite/ without needing to change any JS for them is that bug 284369 screwed up and landed the strings for SeaMonkey without actually letting it use them; bug 442189 is currently fixing that (and some other missing parts), so once that "fixes" it, then you can *fix* it.

(And when you do, please put a full stop at the end of CantDecryptBody1, and fix up the "NOTE To translater" to be a proper LOCALIZATION NOTE that doesn't talk about not translating HTML that isn't there anymore.)

Comment 8

[Hendrik Maryns]

(In reply to comment #7)
> (From update of attachment 326112 [details] [diff] [review])
> Nice tangle: the only reason you can get away with changing the strings in
> /suite/ without needing to change any JS for them is that bug 284369 screwed up
> and landed the strings for SeaMonkey without actually letting it use them; bug
> 442189 is currently fixing that (and some other missing parts), so once that
> "fixes" it, then you can *fix* it.

Hm, I don’t get it.  Note that I am a real beginner in JS and this was just my educated guess at how it would work.  I’ll try to contact people over IRC the next time I work on this.

Ah, do you mean the file msgHdrViewSMIMEOverlay.js is absent in suite?  OK, but that means I’d only have to copy what I did there to the upcoming new one, right?

So I wait for bug 442189.

> (And when you do, please put a full stop at the end of CantDecryptBody1, and
> fix up the "NOTE To translater" to be a proper LOCALIZATION NOTE that doesn't
> talk about not translating HTML that isn't there anymore.)

Oops, ok.

Comment 9

[Phil Ringnalda (:philor)]

Yeah, except it's even more confusing than that: things that are in mail/ are only built by Thunderbird, things that are in suite/ are only built by SeaMonkey, but things that are in mailnews/ may be built by both, or may be only built by SeaMonkey, like http://mxr.mozilla.org/mozilla/source/mailnews/extensions/smime/resources/content/msgHdrViewSMIMEOverlay.js is.