963901 - Variable format error in reportURInotHttpsOrHttp label

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Francesco Lodolo [:flod]]

+++ This bug was initially created as a clone of Bug #843311 +++

http://hg.mozilla.org/mozilla-central/rev/84802a40e62d
reportURInotHttpsOrHttp = The report URI (%1$) should be an HTTP or HTTPS URI.

This doesn't look right to me. This "%1$" should be either "%$" or "%1$S".

If I'm correct, at this point (already landed on mozilla-central) we need a new string ID to fix it.

Comment 1

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] from comment #0)
> This doesn't look right to me. This "%1$" should be either "%$" or "%1$S".

I meant "%S" or "%1$S".

Comment 2

[Francesco Lodolo [:flod]]

Garrett, can you confirm the issue here? I'd like to fix this before it slips to Aurora (if we're still in time)

Comment 3

[Garrett Robinson [:grobinson]]

Attachment: 963901-variable-format-error

Here's the (trivial) patch.

Comment 4

[Garrett Robinson [:grobinson]]

Francesco, I think we can just assume that is an error as it does not conform to the format string syntax. I can try to review it later, but am waiting on a slow build for something else atm so can't confirm for some time.

What do you mean when you say we'll need a new string ID?

Comment 5

[Sid Stamm [:geekboy or :sstamm]]

Comment on attachment 8367645 [details] [diff] [review]
963901-variable-format-error

r=me so long as the change that introduced this string has not been lifted to Aurora yet.  New string name is required if it needs to go to localization again, which I think happens once we .

Axel: is my assumption right?  Is this change OK without a new string label?

Comment 6

[Francesco Lodolo [:flod]]

Comment on attachment 8367645 [details] [diff] [review]
963901-variable-format-error

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

Wrong assumption
https://developer.mozilla.org/en-US/docs/Making_String_Changes

You need a new ID after the string has been exposed to the localization process, which means landed in mozilla-central. In this case I think the original string is broken, not showing the expected parameter, so the more important to make people notice the updated content, and using a new ID is the only way (invalidate existing translation, force localizers to translate the new string).

Aurora is string frozen, which mean you can't change (add/remove) strings at all.

Comment 7

[Francesco Lodolo [:flod]]

We currently have 9 localizations on central. Some of these people are also developers and can understand when a variable is wrong, but we already have 4 wrong localizations too (eo, es-CL, ru, sv-SE).

We either change the ID or commit this and file individual bugs for these locales (I prefer the first option), as long as we do it fast.

If this moves to Aurora, we'll need approvals to fix it and the number of wrong localizations will grow much faster.

Comment 8

[Garrett Robinson [:grobinson]]

Attachment: 963901-variable-format-error

Implement flod's preferred option :)

Comment 9

[Garrett Robinson [:grobinson]]

Attachment: 963901-variable-format-error

Forgot to check "patch"

Comment 10

[Francesco Lodolo [:flod]]

Comment on attachment 8368685 [details] [diff] [review]
963901-variable-format-error

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

Thanks! Patch looks definitely good to me, but I don't think I'm entitled to review it (switching to f+) ;-)

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/d0b0160601f7

Comment 12

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/d0b0160601f7

Comment 13

[Sid Stamm [:geekboy or :sstamm]]

I think we got this sorted out, removing the needinfo flag.