190256 - stringbundles no longer work all over

Status: RESOLVED FIXED

(Core :: Internationalization: Localization, defect)

Description

[Boris Zbarsky [:bzbarsky]]

BUILD: 2003-01-22-08

NOT A PROBLEM IN: 2003-01-21-22

STEPS TO REPRODUCE:

On Linux try simply saving the same file (eg this webpage) twice in a row.  You
will get an uncaught exception in the filepicker when it attempts to confirm
that you want to overwrite the file.  You will not be able to exit the
filepicker with the "OK" button.

The exception you get is:

JavaScript error: 
 line 0: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.formatStringFromName]"  nsresult:
"0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
XStringBundle.getFormattedString() :: getFormattedString :: line 2"  data: no]

Similar exception in bug 190225

ADDITIONAL INFORMATION: backing out bug 121341 fixes the problem.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: patch

This fixes a couple of obvious and egregious errors, and fixes this insiduous
trailing whitespace thing... the old code used to strip trailing (and leading,
but the new code does leading too) whitespace off both keys and values.

I don't like using Trim() there any more than you do (I copied it from the old
code), but the alternative would be to keep both an ident pointer and a
whitespace pointer and switch between the two or something... and I'm far too
tired to work that out right now.  ;)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, that patch fixes all the dependencies except bug 190215 (which I can't
reproduce to start with, which is why I can't claim it's fixed).

Comment 3

[Boris Zbarsky [:bzbarsky]]

> +  // if we're still parsing the value, then append whatever we have..

That should be "the key" of course, not the value... whoever lands it, please
fix before checking in, ok?

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 112380 [details] [diff] [review]
patch

sr=jst

Comment 5

[R.K.Aa.]

*** Bug 190264 has been marked as a duplicate of this bug. ***

Comment 6

[Alec Flett]

Comment on attachment 112380 [details] [diff] [review]
patch

r=alecf, but I'm not sure that Trim is necessary... everything else looks right
though.

Comment 7

[jag (Peter Annema)]

Comment on attachment 112380 [details] [diff] [review]
patch

Marking alecf's +, checking in.

Comment 8

[jag (Peter Annema)]

Hmm.. So it seems to fix the save page case. It doesn't seem to fix the
following case though: go to a site where you can log in, make sure password
manager doesn't know about the values yet so it offers you to remember them.
This dialog shows up with no text, a yes, an empty and a no button.

Holding off on checking the patch in.

Comment 9

[Alec Flett]

its still the right patch - maybe there are other bugs, but this patch should
still land...

Comment 10

[R.K.Aa.]

this patch also fix bug 190167

Comment 11

[Malcolm Rowe]

Apologies if this is a stupid question, but..

It doesn't appear to me that the new properties code handles DOS end-of-line 
sequences (\r\n) correctly, though it looks like UNIX (\n) and MAC (\r) line-
endings would both work.

Did the old code handle this? Do we have any stringbundles that use DOS line-
endings?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Alec, the Trim (or equivalent) is very much necessary; without it the "Add
Language" stuff from bug 190074 does not work.

Jag, the password manager dialog worksforme (with the patch)... Not sure what's
up there...

Malcolm, alec's code handles \r\n fine.  The \r is taken as end-of-line, the \n
is skipped as whitespace.

Comment 13

[Robert Kaiser]

So, it seems the patch has been checked in - and it fixed lots of cases, as far
as I can see.
Thanks for doing this, as also my German localized version of about:plugins is
now working - I guess it broke before because the translation tool escaped ':'
as '\:' in some URL...

Shouldn't this be marked FIXED then - or are still some reproducible issues
left? The only issues I can see with my current build are happening because of
bug 190283

Comment 14

[Simon Fraser [no longer active]]

Marking fixed to get off the blocker list, since alecf checked in the patch this
morning.

Comment 15

[Alec Flett]

(yeah sorry about the FIXED delay - bugzilla was so slow that I had to leave for
a meeting before it finished with the reassignment!)

Comment 16

[Alec Flett]

*** Bug 190118 has been marked as a duplicate of this bug. ***

Comment 17

[David Baron :dbaron:]

Could the appropriate dependencies also be marked fixed?  (I'm adding bug 190083
to the list as well.)

Comment 18

[Boris Zbarsky [:bzbarsky]]

*** Bug 190338 has been marked as a duplicate of this bug. ***