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

VERIFIED FIXED in mozilla2.0b7

Status

()

VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: Unfocused, Assigned: mossop)

Tracking

Trunk
mozilla2.0b7
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

(Whiteboard: [strings])

Attachments

(2 attachments, 1 obsolete attachment)

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?

Comment 2

8 years ago
I'd need a code-reference or a screenshot or something to say.
Created attachment 464679 [details]
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.

Comment 5

8 years ago
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?
(Assignee)

Comment 6

8 years ago
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)

Updated

8 years ago
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.

Comment 9

8 years ago
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.
(Assignee)

Updated

8 years ago
blocking2.0: --- → ?
(Assignee)

Comment 10

8 years ago
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+

Comment 12

8 years ago
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?
(Assignee)

Updated

8 years ago
Whiteboard: [strings]

Comment 15

8 years ago
<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?

Comment 16

8 years ago
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>)
(Assignee)

Comment 17

8 years ago
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).

Comment 18

8 years ago
I think this evaluation is fair.  I tested Force RTL against an en-US build, and things seem fine...
(Assignee)

Comment 19

8 years ago
(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.
(Assignee)

Comment 20

8 years ago
(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.

Comment 21

8 years ago
I tend to agree. Still sounds like there'd be follow-ups, do we have those on file and triaged for string freeze?
(Assignee)

Comment 22

8 years ago
(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?

Comment 23

8 years ago
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?
(Assignee)

Comment 24

8 years ago
(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
(Assignee)

Comment 25

8 years ago
(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)

Comment 26

8 years ago
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?
(Assignee)

Comment 28

8 years ago
(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.
(Assignee)

Comment 30

8 years ago
Created attachment 473636 [details] [diff] [review]
patch rev 1

How does this read?
Attachment #473636 - Flags: review?(l10n)

Comment 31

8 years ago
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+
(Assignee)

Comment 32

8 years ago
Created attachment 473638 [details] [diff] [review]
patch for checkin

Done, for checkin
Attachment #473636 - Attachment is obsolete: true
Attachment #473638 - Flags: review+
Pushed to mozilla-central
http://hg.mozilla.org/mozilla-central/rev/bd1e6e73c594
Status: NEW → RESOLVED
Last Resolved: 8 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-
(Assignee)

Comment 35

8 years ago
No, no need
Flags: in-testsuite? → in-testsuite-
You need to log in before you can comment on or make changes to this bug.