874014 - Tab category titles unreadable in internationalized build

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Gian-Carlo Pascutto [:gcp]]

Attachment: Screenshot of tab titles.

See attached screenshot.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

I'd happier to revert back to icons

Comment 2

[Ian Barlow (:ibarlow)]

I would be happier if we could look at tabs that adjust their width to their content...

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Ian Barlow (:ibarlow) from comment #2)
> I would be happier if we could look at tabs that adjust their width to their
> content...

Basd on the screenshot, we'll be pushing the limit if the text is all on one line.

Comment 4

[Ian Barlow (:ibarlow)]

A few suggestions off the top of my head: 

1. Can we reach out to our localizers, and as if there a shorter word for synced in Durtch than gesynchronizeerd?
2. Can we look at the length of the tab titles, and fall back to icons in cases where the length exceeds what will fit in the title bar? We already do this in landscape mode on tablets, where we don't have space for text. 
3. We will still need variable width tabs, since we're cutting off words even when there is clearly enough space in the title bar: https://bugzilla.mozilla.org/show_bug.cgi?id=867566

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Sriram - Need some direction here. This code is now in Aurora.

Comment 6

[Sriram Ramasubramanian [:sriram]]

I've been trying to fix this for 2 days now.

As per Ian's final decision
1. Have variable-width tab indicators.
2. See if the text fits in there.
3. If not, fit icons in there.

My solutions works most of the time. There are edges where it fails. I'm not happy about trying to do the measure pass of LinearLayout. But that would be the perfect solution for this problem. https://github.com/android/platform_frameworks_base/blob/master/core/java/android/widget/LinearLayout.java#L958

The reason behind using that is, we don't know the size of the dividers, and they play a part here in making the text go to next line -- in edge cases.

Also, doing extra passes of measure() on 3 TextViews is regressing a bit. I'll try to cleanup my patch and post WIP to analyze.

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Guys, if there's no fix for this, can we revert instead of making the non-English speaking world look like second-class citizens?

Comment 9

[Ian Barlow (:ibarlow)]

Yes. I believe we agreed a couple of weeks ago to revert back to icons.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Sriram - Let's get the backout (or new patch) on this soon

Comment 11

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

This reverts back to icon based titles. I tried backing out. But since there were so many changesets in between, I resorted to writing a "backout" patch.

This has everything as the older version. https://hg.mozilla.org/integration/mozilla-inbound/rev/14be1f919f01

Two changes:
1. Removal of unnecessary mContext.
2. There is no style named "AddressBar.ImageButton" <-- which wasn't needed.

(Using either/or operartor in review flag ;). Given that mfinkle is on vacation, if margaret can get back to this sooner, I can push it).

Comment 12

[:Margaret Leibovic]

Comment on attachment 765696 [details] [diff] [review]
Patch

I compared this to the original changeset that landed, and it looks good to me. Also built with this and can confirm it works :)

Comment 13

[Sriram Ramasubramanian [:sriram]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/65f68cc061ec

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/65f68cc061ec

Comment 16

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 765696 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 862996
User impact if declined: Titles will run into 2 lines on some locales.
Testing completed (on m-c, etc.): Landed in m-c on 06/21. This is just a backout and the same code exists on beta and release channels.
Risk to taking this patch (and alternatives if risky): None.
String or IDL/UUID changes made by this patch: None.

Comment 17

[Sriram Ramasubramanian [:sriram]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4249f2cbec7f