554592 - Install buttons are teh borken in RTL languages

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect, P2)

Description

[Fred Wenzel [:wenzel]]

Attachment: Install buttons in Hebrew

See screenshot.

Comment 1

[Wil Clouser [:clouserw]]

If they look good with an RTL language on them, I'm not worried about this bug.  The string "Add to Firefox" should exist, so maybe that's bug 554337

Comment 2

[Jeff Balogh (:jbalogh)]

There's definitely issues with RTL.  Here's a button with some text I stole from amo/ar/: http://grab.by/3pbQ.

Comment 3

[Craig Cook (:craigcook)]

(In reply to comment #2)
Sorry, that's an artifact from the previous version of the buttons when the border was on a span, before we sprited the icons onto an empty b. It's a quick fix:

.html-rtl .button b, 
.html-rtl button b {
    margin: 0 -.5em 0 .6em;
    border-right: 0;
    border-left: 2px groove rgba(150,150,150,0.35);
}

However, that only fixes it if the entire button label is actually in a RTL language. If it's still in English and you just switch the document's direction, the rendering still goes wonky. Or if the button label mixes a RTL language with a LTR language, the rendering is similarly wonky.

Specifically it has to do with the style and position of the empty <b></b> element that attaches the icon. Simply swapping the direction of the document doesn't change where that element falls in the source order, so the browser wants to move it to the right as part of a straight direction flip, but seemingly gets tripped up when the direction of the actual text doesn't match the document's declared direction. 

Ideally, the <b></b> needs to always come "before" the text label, which might be before or after it in the source order, depending on the language direction:

LTR: <b></b> Add to Firefox
RTL: הוסף פיירפוקס <b></b>

Maybe that empty element should just be part of the localized string? That might make it easier to handle the flipping.

I've tried to fix the RTL buttons with CSS only, but I don't think it's entirely a CSS issue, I think it's a browser rendering issue (Webkit doesn't do it, Opera has similar problems, and I need to retest in IE). And so far I've only tested in browsers and OSes localized for US English, so I'm not even sure if this would happen the same way on a system/browser configured for Arabic. 

Simply hiding the <b></b> with display:none; does solve the rendering bug and the buttons still look all nicely rounded and shaded. That's a surefire simple fix, but we lose the icon. That may be an acceptable compromise in the long run.

Another sure fix would be to ensure the buttons contain only properly encoded directional text for any RTL locale, but that may not be practical if the buttons must sometimes contain mixed languages. Currently, AMO shows "Add to ____" for RTL languages where only "___" is localized and "Add to" is always in English. If anything, it's the instructional "Add to" part that should be localized, since brand names like "Firefox" or "Thunderbird" could always appear in English and still be recognized.

So long story short, it's a tricky localization issue and we might want to consult Seth B on how to approach it. If the whole label is localized everything works and the bug fixes itself, but if the label needs to mix languages we need to sort out just how best to do that (maybe needs an inline <bdo> element for the English bits?). Once we know which way we're going we can definitely fix the rendering, even if it means sacrificing the icons in RTL.

Comment 4

[Craig Cook (:craigcook)]

Any word on this? I'm not sure what I should do to fix this bug without some further input on how to address localization. The options as I see it:

1. Ensure that the entire text label of any button in a RTL language is entirely encoded in said language, with no mixed-languages cases.

2. If mixed labels are required, find some other solution:
  2a. Simply remove the icons in RTL languages.
  2b. Some other fix that would allow us to keep the icons, but I don't know what that is yet.

Comment 5

[Wil Clouser [:clouserw]]

ehsan might have an idea for this, although it might be the same issue as bug 542909.  Ehsan?

Comment 6

[(no longer active)]

I can't reproduce this, but I doubt that it's the same issue as bug 542909.  Am I missing something on how to reproduce this?

Comment 7

[Wil Clouser [:clouserw]]

What I see on https://preview.addons.mozilla.org/z/he/firefox/ isn't nearly that bad.  A light vertical line on the "Continue to Download" and the regular download buttons have a vertical line that is a little too long.  But not like your screenshot.  wenzel:  can you confirm this is improved or were you looking at a different page?

Comment 8

[Wil Clouser [:clouserw]]

Attachment: screenshot

Oh, I took a screenshot too and forgot to attach it

Comment 9

[Fred Wenzel [:wenzel]]

Yes, much better. I looked at an add-on details page initially, and that looks better now too.

On the front page, the install buttons are aligned all weirdly, but that's a different bug.

I think once we get rid of the two minor line issues you mention in comment 7, this can be called fixed.

Comment 10

[(no longer active)]

I tried inspecting the element in Firebug, but the featured addons box just keeps swiping pages! :-)  Is there any other place on AMO where that button shows up?

Comment 11

[Craig Cook (:craigcook)]

That teeny bit of border overflow seems to be a slight variance in rendering between Windows and Mac (fonts are likely to blame). Getting it perfect on Mac is a bit too much on Windows, and perfect on Windows isn't quite enough on Mac. So I've adjusted the padding to split the difference and it looks pretty good on both now (better to have too little rather than too much). I don't have a Linux box to check on so I'm just crossing my fingers there.

Padding tweaked in r65959, and also removed the stray border on the EULA buttons.

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Post-fix screenshot

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on https://preview.addons.mozilla.org/z/he/firefox/addon/1865/; checked this on Mac, Windows, and Linux Firefox 3.6.3 builds.