Closed Bug 1267251 Opened 5 years ago Closed 5 years ago

Rename strings landed in dom.properties to reflect new content

Categories

(Core :: DOM: Animation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: flod, Assigned: hiro)

References

Details

Attachments

(1 file)

Tentatively assigning to the same person who fixed bug 1255683
Assignee: nobody → bbirtles
I'd take this.
Francesco, let me clarify what we should do here.  Changing all IDs in that changeset is enough to do here?  I've added another string in https://hg.mozilla.org/mozilla-central/rev/f4f185cfc034.  Can we also change the ID here?
Assignee: bbirtles → hiikezoe
Flags: needinfo?(francesco.lodolo)
Thanks for taking the bug.

There's no need to change the ID for a newly added string (unless you have code that requires that, for example generating automatically the ID of a string) 
https://hg.mozilla.org/mozilla-central/rev/f4f185cfc034

On the other hand, you'd need to change the string IDs that landed in
https://hg.mozilla.org/mozilla-central/rev/543eb9757c11

So, yes, updating all those 7 IDs in the .properties file and code would fix this bug. Make also sure to update reference in localization comments.

Using a new string ID is the only way to make sure existing (obsolete) localizations won't be used.
Flags: needinfo?(francesco.lodolo)
(In reply to Francesco Lodolo [:flod] from comment #3)
> Thanks for taking the bug.
> 
> There's no need to change the ID for a newly added string (unless you have
> code that requires that, for example generating automatically the ID of a
> string) 
> https://hg.mozilla.org/mozilla-central/rev/f4f185cfc034

Thanks for the explanation.  I guess changing the new ID lets localizers a redundant work, but I'd actually like to change this new ID for consistency with other IDs.  Should we leave it?
> Thanks for the explanation.  I guess changing the new ID lets localizers a
> redundant work, but I'd actually like to change this new ID for consistency
> with other IDs.  Should we leave it?

If it's just for consistency, I would leave the existing string (we're pretty creative and inconsistent when it comes to string IDs).
Brian, The only way what comes in my mind is to prepend 'Compositor' to the IDs. Or append 'OnCompositor'?  What do you think of?
Flags: needinfo?(bbirtles)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)
> Brian, The only way what comes in my mind is to prepend 'Compositor' to the
> IDs. Or append 'OnCompositor'?  What do you think of?

I don't think it matters. Prepending Compositor is probably simplest.

(At the same time, I don't think the meaning of the strings actually changed. It would be interesting to see what localizations for these strings exist and if they actually need to be changed. We get pretty loose translations in Japanese such as "reflow" being translated as "render" -- so if other translations are translating "async" as something equivalent to "on the compositor" then they don't need to change. I suspect any Western-language translations, however, are probably closer.)
Flags: needinfo?(bbirtles)
(In reply to Brian Birtles (:birtles) from comment #7)
> (At the same time, I don't think the meaning of the strings actually
> changed. It would be interesting to see what localizations for these strings
> exist and if they actually need to be changed. We get pretty loose
> translations in Japanese such as "reflow" being translated as "render" -- so
> if other translations are translating "async" as something equivalent to "on
> the compositor" then they don't need to change. I suspect any
> Western-language translations, however, are probably closer.)

Even if the meaning is more or less the same, both wording and content are different (no more reference to “aFrames” for instance). This is also creating inconsistencies with the string that has landed a bit after (AnimationWarningContentTooSmall).

FWIW, I’d definitely update my French translations, thus I’d like to be warned of this change.
Patrick, can you please take a look changes in devtools?  The new message ID is too long to fit in 80 chars,  so I needed a temporary variable is there.
Comment on attachment 8754196 [details]
MozReview Request: Bug 1267251 - Change message IDs for animation warnings. r?birtles,pbro

https://reviewboard.mozilla.org/r/53828/#review50530
Attachment #8754196 - Flags: review?(bbirtles) → review+
Comment on attachment 8754196 [details]
MozReview Request: Bug 1267251 - Change message IDs for animation warnings. r?birtles,pbro

https://reviewboard.mozilla.org/r/53828/#review50580
Attachment #8754196 - Flags: review?(pbrosset) → review+
Comment on attachment 8754196 [details]
MozReview Request: Bug 1267251 - Change message IDs for animation warnings. r?birtles,pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53828/diff/1-2/
Comment on attachment 8754196 [details]
MozReview Request: Bug 1267251 - Change message IDs for animation warnings. r?birtles,pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53828/diff/1-2/
https://hg.mozilla.org/mozilla-central/rev/a66141bd566a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.