Closed Bug 906273 Opened 6 years ago Closed 6 years ago
[B2G] [Fonts] Improve display of Arabic app names on Firefox
368.46 KB, image/jpeg
407.81 KB, image/jpeg
46 bytes, text/x-github-pull-request
|Details | Review|
430.30 KB, image/png
612 bytes, patch
|Details | Diff | Splinter Review|
402.11 KB, image/png
The standard font used in B2G has readability issues. The titles of home screen icons are hard to read given the combination of default font, font size used and the phone screen resolution. We need to add better fonts and also tweak the font sizes is B2G apps. Possibly related to #775308 https://bug893530.bugzilla.mozilla.org/attachment.cgi?id=775308
Bug 843592 is fixed now. Is there more to do here? If we need additional fonts, which do we need to add?
Will retest with nightly and give the feedback here.
Arabic Font Test on Geeksphone Nightly and gaia master branch. Fonts are much more readable.
So can we resolve this then?
(In reply to Axel Hecht [:Pike] from comment #4) > So can we resolve this then? Perhaps I need to test further on Unagi/Keon class device before we resolve this.
I can’t comment on the hard to read part without seeing it at its actual resolution/screen, but I see that the descenders of some letters are cut off and this probably because the Arabic font (Droid Arabic Naskh, it seems) has larger descender that the Latin font. BTW, there are several (though not major) issues with Droid Arabic Naskh, and I forked it to fix them: https://github.com/khaledhosny/sahl-naskh (upstream already aware of them and the fork, but things are bit slow on their side).
Testing Arabic Font on Ungai. The homescreen icon label are still hard to read. The characters are cut off at the bottom Test Device: B2G:Nightly build mozilla-central-unagi-eng Gaia: Master
Need an expert option. The quality issues seems to be due to a combination of screen resolution and font size. What do you reckon?
Need an expert opinion. The quality issues seems to be due to a combination of screen resolution and font size. What do you reckon?
If I'm reading the Gaia homescreen CSS (see gaia/apps/homescreen/style/grid.css) correctly, it looks like it is using a font size of 1.3rem: https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/grid.css#L100 and setting the height of the icon labels to 1.9rem: https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/grid.css#L123 Also, it uses overflow:hidden, so that any text that extends outside this height will not be visible: https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/grid.css#L85 This implies that if the overall height of the glyphs in any font that's used (not just the default Fira Sans, but any font that fallback chooses - such as for Arabic) exceeds about 1.46em, something's going to be clipped. Looking at DroidArabicNaskh, it uses an em-square of 2048 units, but the maximum ascent/descent appear to be 2474 and -1437, for a total height of 3911 units or 1.9em. Which is too tall to fit within the height defined for the icon labels. So certain particularly tall and/or deep glyphs are going to be cut off. So I think what needs to happen here is to relax the tight constraints on the label sizes in some way - either by allowing them more height, or by not using overflow:hidden at least in the y-direction. Arabic is almost certainly not the only non-Latin script that will encounter this issue.
Thanks for the feedback Jonathan. :) Perhaps we need to reassign this bug to Homescreen component. Any thoughts Vivien?
I tried to fix this by just setting overflow-y:visible for the icon labels, which in principle seems like it should avoid the clipping. However, this doesn't work (at least not in the way I expected; maybe there's a good reason for it). I filed bug 939830 on that. In the meantime, we should be able to adjust the height of the label box to work around the issue here.
The container height for app names is defined here: https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/style/grid.css#L123 and this is the issue thought
Moving to Gaia:Homescreen (and cancelling needinfo? on :vingtetun, as I've gone ahead with this). Also updating the bug summary to refer specifically to the homescreen app labels, as that's the particular issue we'll aim to fix here. (If there are more general issues with display of Arabic text throughout FxOS, then let's have a separate bug for that.) A related issue with the current homescreen styling is that the presence of any Arabic characters in an app name will shift the baseline of the text down by several pixels. This is visible in attachment 831451 [details] and attachment 831487 [details], though it's more obvious if you create an icon that has a mixed Latin + Arabic label, so that it's easier to compare the baseline to adjacent apps. I'm experimenting with a patch that should fix both the clipping and baseline-shift issues, if all goes well.
Summary: [B2G] [Fonts] Improve readability of Arabic on B2G → [B2G] [Fonts] Improve display of Arabic app names on FirefoxOS homescreen
Assignee: nobody → jfkthame
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Three screenshots of the Homescreen app on Unagi, for comparison. Center: the icon grid as displayed by current trunk build, for reference. Left: with a couple of Arabic letters appended to each app name that starts with "M". Note that (a) the labels are displaced downwards, and (b) the descender of the ج is truncated. Right: a build with the above patch, and with the same Arabic letters added to the "M" labels. Note that (a) the baseline of the text remains correct, and (b) the descender of ج is displayed fully.
Comment on attachment 8334085 [details] [review] patch, increase height of app label box, but ensure text stays on the current baseline The code looks good to me, this is r+ although you have to add my patch  before landing yours in order to adapt the scale effect in drag&drop according to your changes in grid. Mix them and merge, thanks a lot  https://bug906273.bugzilla.mozilla.org/attachment.cgi?id=8334394
Attachment #8334085 - Flags: review?(crdlc) → review+
I've updated the PR to include your drag-n-drop patch (thanks).... at least I think so (as a git[hub] novice, I'm never entirely sure I'm doing things right!). I don't have commit access to merge this AFAIK, so if you can take care of that side of things it'd be great.  https://github.com/mozilla-b2g/gaia/pull/13814
Merged in master: https://github.com/mozilla-b2g/gaia/commit/dfc678d977c7da79de798b176add5b69b6838801 Thanks Jonathan
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Mass Edit: adding the [rtl-meta]
This issue verified successfully on Flame 2.2 Gaia-Rev 7c5b27cad370db377b18a742d3f3fdb0070e899f Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ce27f2692382 Build-ID 20150115002505 Version 37.0a2 Reproduce rate 0/5
You need to log in before you can comment on or make changes to this bug.