Closed Bug 963901 Opened 7 years ago Closed 7 years ago

Variable format error in reportURInotHttpsOrHttp label


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

Not set





(Reporter: flod, Assigned: grobinson)




(1 file, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #843311 +++
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)
Attached patch 963901-variable-format-error (obsolete) — Splinter Review
Here's the (trivial) patch.
Attachment #8367645 - Flags: review?(sstamm)
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]

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]

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

Wrong assumption

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)
Attached file 963901-variable-format-error (obsolete) —
Implement flod's preferred option :)
Assignee: nobody → grobinson
Attachment #8367645 - Attachment is obsolete: true
Attachment #8368684 - Flags: review?(francesco.lodolo)
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]

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+
Keywords: checkin-needed
Closed: 7 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.