Variable format error in reportURInotHttpsOrHttp label

RESOLVED FIXED in mozilla29

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: flod, Assigned: grobinson)

Tracking

Trunk
mozilla29
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

+++ 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.
(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".
Garrett, can you confirm the issue here? I'd like to fix this before it slips to Aurora (if we're still in time)
Flags: needinfo?(grobinson)
(Assignee)

Comment 3

4 years ago
Created attachment 8367645 [details] [diff] [review]
963901-variable-format-error

Here's the (trivial) patch.
Attachment #8367645 - Flags: review?(sstamm)
(Assignee)

Comment 4

4 years ago
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?
Flags: needinfo?(grobinson) → needinfo?(francesco.lodolo)
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?
Attachment #8367645 - Attachment is patch: true
Attachment #8367645 - Attachment mime type: message/rfc822 → text/plain
Attachment #8367645 - Flags: review?(sstamm)
Attachment #8367645 - Flags: review?(l10n)
Attachment #8367645 - Flags: review+
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.
Attachment #8367645 - Flags: review?(l10n) → feedback-
Flags: needinfo?(francesco.lodolo)
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.
Flags: needinfo?(sstamm)
(Assignee)

Comment 8

4 years ago
Created attachment 8368684 [details]
963901-variable-format-error

Implement flod's preferred option :)
Assignee: nobody → grobinson
Attachment #8367645 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8368684 - Flags: review?(francesco.lodolo)
(Assignee)

Comment 9

4 years ago
Created attachment 8368685 [details] [diff] [review]
963901-variable-format-error

Forgot to check "patch"
Attachment #8368684 - Attachment is obsolete: true
Attachment #8368684 - Flags: review?(francesco.lodolo)
Attachment #8368685 - Flags: review?(francesco.lodolo)
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+) ;-)
Attachment #8368685 - Flags: review?(francesco.lodolo) → feedback+
Attachment #8368685 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/d0b0160601f7
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/d0b0160601f7
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla29
I think we got this sorted out, removing the needinfo flag.
Flags: needinfo?(sstamm)
You need to log in before you can comment on or make changes to this bug.