Closed Bug 583965 Opened 14 years ago Closed 14 years ago

Using a postfix string to signify an item is an update may not be l10n friendly

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: Unfocused, Assigned: mossop)

References

Details

(Whiteboard: [strings])

Attachments

(2 files, 1 obsolete file)

Currently, for update items, the "Update" postfix string is a separate element, that doesn't allow l10n formatting. This may not be friendly to some locales that wouldn't normally use a postfix to signify this.
Something we would have to fix before string freeze?
I'd need a code-reference or a screenshot or something to say.
Attached image Screenshot
This is actually showing a disabled addon (probably the same problem), but just imagine "(disabled)" really says "Update".
To clarify: each one of those (name, version, postfix) has its position hardcoded. AFAIK, in the old extension manager, version was also hardcoded to come directly after name.
So, the code in question is really http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions.xml#754, right? The only way I'd know to fully localize would be to put all of the children of name-container into a localizable entity. Not really l10n friendly, either. Or, add a direction attribute to name-container? Not sure. Another question, is stuff there accessible to hacks via intl.css?
I suspect the best option here is just to create a bunch of localized strings here, something like: "%S %S (disabled)" "%S %S (blocked)" etc. With the postfixes I know of right now that would mean 8 strings though.
(In reply to comment #6) > I suspect the best option here is just to create a bunch of localized strings > here, something like: > > "%S %S (disabled)" > "%S %S (blocked)" > > etc. > > With the postfixes I know of right now that would mean 8 strings though. Uh, that's a lot. There's also the issue of addons that don't have a version number - need some way of avoiding having a double space. So we'd need to do something like "%S (blocked)", with that token being replaced by either just the name, or another entity that holds both name and version (ie, "%S %S"). Thankfully, we're no longer styling the postfix differently from the rest of that line, so we can do this now.
Assignee: nobody → dtownsend
Blocks: 585950
(In reply to comment #5) > So, the code in question is really > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions.xml#754, > right? Yes. > Another question, is stuff there accessible to hacks via intl.css? I'm not familiar with how that's typically used. We don't currently import intl.css directly, though maybe its imported via some other file. So maybe? Would like to keep that as a last resort though.
It's part of xul.css, so it's included. I just don't know if it propagates into xbl bindings. I would hope it works at the same level of userChrome.css, so you could use that to test. Though, if we're going for an entry per case without markup, using CSS to hack stuff in will obviously not help. But if we can go without styling, just a line of text may be just fine.
blocking2.0: --- → ?
Axel, can you just clarify whether this is something we really need to fix to make this properly localizable or will localizers be able to work around it?
Axel, need your decision to above to decide if this is a beta6+ blocker.
blocking2.0: ? → beta6+
I'd love to get a bit of feedback from localizers, I don't know enough edgecases.
Note that if possible, I'd like to be able to crop the addon name (if its too long), but NOT crop the postfix. Cropping the postfix means we lose important information. I want to have: Blair's Really Awesome Exten... (disabled) Instead of: Blair's Really Awesome Extension (d... (See bug 567652 comment 10.) So unless its absolutely necessary, I want to avoid putting all that line in a localized string.
This is how I solved this in our localization: Test Addon v0.1 Update doesn't work for us, but Test Addon v0.1 (update) does. If we already have Test Addon v0.1 (disabled) Test Addon v0.1 (blocked) why it is Test Addon v0.1 Update?
Whiteboard: [strings]
<ehsan> I think we probably want to wrap things like "(disabled)" in an element which can be set to rtl explicitly <ehsan> for add-on names, descriptions, etc we want to look at the first character and see if it's an RTL character or not <ehsan> and display the strings in span elements with the correct direction based on that <ehsan> so, something like this should work: <ehsan> <div dir=rtl><span id=foo>Add-on Name</span> <span dir=rtl>(disabled)</span></div> <ehsan> where we set #foo's dir explicitly based on its textContent's first character <ehsan> we might need to stick an RLM before the opening parenthesis though * Pike copies and pastes that into the bug That looks like we should stick to elements to separate the flag strings. Also, I am too a bit confuzzled about the update postfix, and it's l10n note. And, in particular if we go for things in braces and things without, the order in which they show up should probably match. Like, right now, the line could be Foo addon (disabled) Update right? Looks confusing, though it may just very well be totally unrealistic to have that string. Does that help?
Mossop asked me to explain what I meant in comment 15. The main problem here is that the "(disabled)" string might be in RTL and "Add-on Name" might be in LTR (or vice versa). We know the directionality of the former (which is determined by the locale's direction), but we don't know that of the latter. The direction of the add-on name (and its description and whatever other textual data we retrieve for it from AMO) can be determined by looking at the first character (well, this is somewhat simplistic, but it should be enough) and check to see if it's in this range using this formula (assuming c is a character code): (((0x0590 <= (c)) && ((c)<= 0x05FF)) || (((c) >= 0xfb1d) && ((c) <= 0xfb4f))) || ((0x0600 <= (c)) && ((c)<= 0x06FF)) (This is basically IS_HEBREW_CHAR(c) || IS_ARABIC_CHAR(c) stolen from <http://mxr.mozilla.org/mozilla-central/source/intl/unicharutil/util/nsBidiUtils.h#274>)
Ok I don't think there is an RTL problem here then. The actual XUL here is roughly this: <hbox> <label>Add-on name</label> <label>Add-on version</label> <label>(disabled)</label> </hbox> The labels will display ltr or rtl depending on the locale and the hbox will flip direction too so this seems to work out unless I am missing something? The only problem is that as Ehsan mentions the add-on name may be a different direction to the locale's. This isn't great but is already a problem in 3.6 so I wouldn't consider it a blocker for 4.0 (and really should be fixed by making XUL label's direction-aware).
I think this evaluation is fair. I tested Force RTL against an en-US build, and things seem fine...
(In reply to comment #14) > This is how I solved this in our localization: > > Test Addon v0.1 Update doesn't work for us, but > Test Addon v0.1 (update) does. > > If we already have > Test Addon v0.1 (disabled) > Test Addon v0.1 (blocked) > why it is > Test Addon v0.1 Update? Spoke with Boriss about this. The "(disabled)" and "(blocked)" postfixes are really states for installed add-ons. The "Update" postfix describes the thing you are looking at: "Foo 1.0 Update" is the update to Foo 1.0. It should never appear along with the other postfixes since updates won't be disabled or blocked. That She said it would also be fine to do it as you have done, we could even change it for English if that makes more sense for all locales to match up.
(In reply to comment #19) > She said it would also be fine to do it as you have done, we could even change > it for English if that makes more sense for all locales to match up. In fact unless we want to change the default locale in this way I think we can call this bug WFM. I've not yet heard of real problems here and keeping the postfixes allows us to do the wrapping properly as Blair mentions.
I tend to agree. Still sounds like there'd be follow-ups, do we have those on file and triaged for string freeze?
(In reply to comment #21) > I tend to agree. Still sounds like there'd be follow-ups, do we have those on > file and triaged for string freeze? Which follow-ups are you referring to?
Whether it's (Update) or Update, and if Update for itself needs a different string markup. Because, if that'd be really different, it might. I really have a hard time to discuss (Update) vs Update as I haven't seen it in real life yet. Can I haz screencats?
(In reply to comment #23) > Whether it's (Update) or Update, and if Update for itself needs a different > string markup. Because, if that'd be really different, it might. > > I really have a hard time to discuss (Update) vs Update as I haven't seen it in > real life yet. Can I haz screencats? http://grab.by/6ikk
(In reply to comment #24) > (In reply to comment #23) > > Whether it's (Update) or Update, and if Update for itself needs a different > > string markup. Because, if that'd be really different, it might. > > > > I really have a hard time to discuss (Update) vs Update as I haven't seen it in > > real life yet. Can I haz screencats? > > http://grab.by/6ikk (I should probably file a bug on that showing as incompatible, it isn't meant to I think)
We talked about this on irc a bit more, and want to make the localization note a it more clear on the update postfix that offers () or somesuch for languages for which "foo 1.x Update" doesn't linguistically resolve to an update to that version.
Where does that put us in terms of b6? Is there string-freeze-sensitive change required here, or not?
(In reply to comment #27) > Where does that put us in terms of b6? Is there string-freeze-sensitive change > required here, or not? I'm going to file the patch for the localisation note today and assuming Pike is happy with it it can also land today. I think we probably want localisers to see the message before they localise so it probably still wants to block b6.
Thanks, Dave - sounds good.
Attached patch patch rev 1 (obsolete) — Splinter Review
How does this read?
Attachment #473636 - Flags: review?(l10n)
Comment on attachment 473636 [details] [diff] [review] patch rev 1 Looks good to me. The intended version number for update.postfix is the one to update to, right? Then it might be sweet to use "<Addon name> <1.1> Update" to make that more obvious? r=me either way.
Attachment #473636 - Flags: review?(l10n) → review+
Done, for checkin
Attachment #473636 - Attachment is obsolete: true
Attachment #473638 - Flags: review+
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b6
Verified fixed with an fa build of Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100916 Firefox/4.0b7pre Dave, I assume we do not want to have any automated test?
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Flags: in-litmus-
No, no need
Flags: in-testsuite? → in-testsuite-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: