Closed
Bug 874014
Opened 11 years ago
Closed 11 years ago
Tab category titles unreadable in internationalized build
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect)
Tracking
(firefox23 verified, fennec23+)
VERIFIED
FIXED
Firefox 24
People
(Reporter: gcp, Assigned: sriram)
References
Details
Attachments
(2 files)
200.00 KB,
image/png
|
Details | |
7.62 KB,
patch
|
Margaret
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
See attached screenshot.
Comment 2•11 years ago
|
||
I would be happier if we could look at tabs that adjust their width to their content...
Flags: needinfo?(ibarlow)
Comment 3•11 years ago
|
||
(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•11 years ago
|
||
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•11 years ago
|
||
Sriram - Need some direction here. This code is now in Aurora.
Flags: needinfo?(sriram)
Updated•11 years ago
|
tracking-fennec: --- → ?
Assignee | ||
Comment 6•11 years ago
|
||
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.
Flags: needinfo?(sriram)
Updated•11 years ago
|
Assignee: nobody → sriram
tracking-fennec: ? → 23+
Reporter | ||
Comment 8•11 years ago
|
||
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•11 years ago
|
||
Yes. I believe we agreed a couple of weeks ago to revert back to icons.
Comment 10•11 years ago
|
||
Sriram - Let's get the backout (or new patch) on this soon
Flags: needinfo?(sriram)
Assignee | ||
Comment 11•11 years ago
|
||
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).
Attachment #765696 -
Flags: review?(mark.finkle)
Attachment #765696 -
Flags: review?(margaret.leibovic)
Flags: needinfo?(sriram)
Comment 12•11 years ago
|
||
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 :)
Attachment #765696 -
Flags: review?(mark.finkle)
Attachment #765696 -
Flags: review?(margaret.leibovic)
Attachment #765696 -
Flags: review+
Assignee | ||
Comment 13•11 years ago
|
||
Comment 15•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Assignee | ||
Comment 16•11 years ago
|
||
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.
Attachment #765696 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #765696 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 17•11 years ago
|
||
Updated•11 years ago
|
status-firefox23:
--- → fixed
Updated•11 years ago
|
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•