437405 - [es-ES] installer/overrides.properties newline cleanup

Status: RESOLVED FIXED

(Mozilla Localizations :: es-ES / Spanish, defect)

Description

[Axel Hecht [:Pike]]

In http://mxr.mozilla.org/l10n/search?string=FileError&find=[beimru][deku]%2Fbrowser%2Finstaller&findi=&filter=^[^\0]*%24&hitlimit=&tree=l10n and http://mxr.mozilla.org/l10n/search?string=FileError&find=[ez][sh]-[EC].%2Fbrowser%2Finstaller&findi=&filter=^[^\0]*%24&hitlimit=&tree=l10n, the original markup of the newlines in the error messages got mangled and changed.

I don't see this as a release blocker, as apparently preprocess-locale.pl does 'something', but the newline combo should really look like en-US, i.e., \r\n\r\n$0\r\n\r\n and \r\n elsewhere.

This affects both FileError and FileError_NoIgnore.

Please attach a patch and request approval1.9 to include this in a future 3.0.x release.

Comment 1

[RickieES aka Ricardo Palomares]

I'm the one to blame. MozillaTranslator is exporting the file this way. I'll look into it ASAP. If I see that it will take me longer, I'll provide a patch for this specific problem.

Comment 2

[Axel Hecht [:Pike]]

I was about to ask for a tool bug, now I know :-).

It's nice to see that all the tools floating around find different bugs as they evolve.

Comment 3

[Guillermo López :willyaranda (probably SLOW response)]

The problem is:

63 FileError = Error al abrir el archivo para escribir: 
\n
\n$0
\n

the "$0", isn't it?

Comment 4

[Axel Hecht [:Pike]]

no, the problem is that the original file has, literally, "\r\n\r\n$0\r\n\r\n", or, two windows newlines before and after the $0, and that the \r were actually converted from literal backslash-r to 0xsomething (sorry, don't have the charcode at my fingertips).

Comment 5

[RickieES aka Ricardo Palomares]

(In reply to comment #4)
> no, the problem is that the original file has, literally, "\r\n\r\n$0\r\n\r\n",
> or, two windows newlines before and after the $0, and that the \r were actually
> converted from literal backslash-r to 0xsomething (sorry, don't have the
> charcode at my fingertips).
> 

Actually, it was converted to the literal representation of the escape chars (it is like if, instead of the ASCII chars "\" and "r", the escape char represented by "\r" was in that position).

I've filed bug #439349 to fix this in MT.

Comment 6

[Guillermo López :willyaranda (probably SLOW response)]

Attachment: Patch that solves the problem

Sorry for the delay...

Firefox 3.0.1 or 3.0.2?

Comment 7

[RickieES aka Ricardo Palomares]

(In reply to comment #6)
> Created an attachment (id=328211) [details]
> Patch that solves the problem


I'm afraid the patch doesn't fully solve the problem. Some newlines remain. Compare:

-FileError = Error al abrir el archivo para escribir: 
\n
\n$0
\n
\nPulse Abortar para detener la instalación,
\nReintentar para intentarlo de nuevo, o
\nIgnorar para saltarse este archivo.

with:

+FileError = Error al abrir el archivo para escribir: \r\n\r\n$0\r\n\r\nPulse Abortar para detener la instalación,
\r\nReintentar para intentarlo de nuevo, o
\r\nIgnorar para saltarse este archivo.

which should be:

+FileError = Error al abrir el archivo para escribir: \r\n\r\n$0\r\n\r\nPulse Abortar para detener la instalación,\r\nReintentar para intentarlo de nuevo, o\r\nIgnorar para saltarse este archivo.

As a hint, when seeing the output of cvs -z3 diff -u, the lines marked with "+" shouldn't have any weird character on them.


> Sorry for the delay...
> 
> Firefox 3.0.1 or 3.0.2?


I'm pretty sure we're too late for 3.0.1, so you should ask for approval1.9.0.2.

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 328211 [details] [diff] [review]
Patch that solves the problem

What Ricardo said :-)

Comment 9

[Guillermo López :willyaranda (probably SLOW response)]

I have changed again my working copy and diffing again, and nothing changes, so should I edit the diff manually (remove the two newlines)?

*I think* that the problem is still on MT...

Comment 10

[Guillermo López :willyaranda (probably SLOW response)]

Attachment: New patch without newlines

This should be the good one.

Comment 11

[Axel Hecht [:Pike]]

Comment on attachment 328719 [details] [diff] [review]
New patch without newlines

a=me for landing for 3.0.2. Please check in with a comment referencing this bug and my approval/review, and use the fixed1.9.0.2 and verified1.9.0.2 keywords to track landing and testing.

Comment 12

[Guillermo López :willyaranda (probably SLOW response)]

guillermo@guillermo-laptop:~/MT/firefox/trunk/l10n/es-ES/browser/installer$ ./commit-patch -m "bug443777 and bug 437405. Changed an accesskey, removed newlines and fix codification. Patch by Guillermo, r=[e-mail]" override-patch.diff
Checking in override.properties;
/l10n/l10n/es-ES/browser/installer/override.properties,v  <--  override.properties
new revision: 1.5; previous revision: 1.4
done
guillermo@guillermo-laptop:~/MT/firefox/trunk/l10n/es-ES/browser/installer$ 


http://l10n.mozilla.org/buildbot/builders/linux-langpack/builds/11269

Checked in:

http://bonsai-l10n.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/l10n&subdir=l10n/es-ES/browser/installer&command=DIFF_FRAMESET&root=/l10n&file=override.properties&rev1=1.4&rev2=1.5

And build:

http://l10n.mozilla.org/buildbot/builders/linux-langpack/builds/11269

Comment 13

[Axel Hecht [:Pike]]

Setting fixed1.9.0.2 keyword to that extent.

Could you still verify the fixes, and then change the keyword over to verified1.9.0.2?