Closed
Bug 1432045
Opened 6 years ago
Closed 6 years ago
Text vertical alignment on title bar with non-latin characters mix
Categories
(Firefox :: Theme, defect, P3)
Firefox
Theme
Tracking
()
VERIFIED
FIXED
Firefox 60
People
(Reporter: mte90net, Assigned: dao)
References
Details
(Keywords: regression)
Attachments
(2 files)
175.36 KB,
image/png
|
Details | |
59 bytes,
text/x-review-board-request
|
Gijs
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details |
As you can see in the screenshot, if you are looking on a page that in the title tag contain non-latin characters (in this kind Hindi I guess) and contain latin characters the vertical alignment instead to be on bottom (like for the others) is not on the top. A page to test: https://addons.mozilla.org/en-US/firefox/addon/swecha/ This bug happen with Firefox Nightly 59. I am not sure if happen also on other systems but seems more a rendering issue.
Assignee | ||
Updated•6 years ago
|
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8944680 [details] Bug 1432045 - Maintain consistent alignment in case of font fallback for non-Latin characters. https://reviewboard.mozilla.org/r/214836/#review220680 Tentative r=me in that this wfm when testing on macOS I'm a little concerned about this and bug 1402368 using slightly different constants here, and as far as I can tell, in both cases they're basically eyeballed to be "right". In a way, it feels like we should share them, but on the other hand I guess on Windows (and maybe Linux?) we use radically different font sizes for the url bar vs. tabs so maybe that doesn't make sense. In any case, it's kinda unfortunate that we have to fix it this way and there isn't some kind of vertical-align: baseline or whatever that helps here... Perhaps it's worth talking to the gfx text-y folks like jfkthame if there are more structural solutions we can use?
Attachment #8944680 -
Flags: review?(gijskruitbosch+bugs) → review+
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b529d05259c5 Maintain consistent alignment in case of font fallback for non-Latin characters. r=Gijs
Assignee | ||
Comment 5•6 years ago
|
||
(In reply to :Gijs (lower availability until Jan 27) from comment #3) > Comment on attachment 8944680 [details] > Bug 1432045 - Maintain consistent alignment in case of font fallback for > non-Latin characters. > > https://reviewboard.mozilla.org/r/214836/#review220680 > > Tentative r=me in that this wfm when testing on macOS > > I'm a little concerned about this and bug 1402368 using slightly different > constants here, and as far as I can tell, in both cases they're basically > eyeballed to be "right". In a way, it feels like we should share them, but > on the other hand I guess on Windows (and maybe Linux?) we use radically > different font sizes for the url bar vs. tabs so maybe that doesn't make > sense. In any case, it's kinda unfortunate that we have to fix it this way > and there isn't some kind of vertical-align: baseline or whatever that helps > here... Perhaps it's worth talking to the gfx text-y folks like jfkthame if > there are more structural solutions we can use? jfkthame, any further thoughts on this? See also bug 1409615 comment 4.
Flags: needinfo?(jfkthame)
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b529d05259c5
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Comment 7•6 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #5) > jfkthame, any further thoughts on this? See also bug 1409615 comment 4. Not really. AFAICT something like vertical-align doesn't have any effect here. Given the potential for very different font sizes and relative ascent/descent, especially when font fallback kicks in for an "obscure" character that isn't supported by the main UI font, I think all we can do is to aim for what looks right, and try to avoid designs where text is tightly boxed so that if an unusually tall or deep glyph occurs, it'll run out of room. In general, the line height and vertical extent of UI elements needs to be substantially larger than nominal font size to minimize the risk of such issues; but of course making it too large may look bad for the usual case where there's just simple Latin text. So it's a balancing act...
Flags: needinfo?(jfkthame)
Comment 8•6 years ago
|
||
I tested with https://addons.mozilla.org/en-US/firefox/addon/swecha/ on Firefox 58.0 and today's Nightly 60.0a1. The problem exists on Firefox 58, but fixed (and looks much better) on Nightly 60. Tested with Ubuntu. [bugday-20180131]
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
status-firefox58:
--- → wontfix
Assignee | ||
Comment 9•6 years ago
|
||
Comment on attachment 8944680 [details] Bug 1432045 - Maintain consistent alignment in case of font fallback for non-Latin characters. Approval Request Comment [Feature/Bug causing the regression]: unclear. it's possible that this regressed with photon in 57. [User impact if declined]: see comment 0 [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: / [Is the change risky?]: no [Why is the change risky/not risky?]: fairly simple fix, baked in Nightly for two weeks [String changes made/needed]: /
Attachment #8944680 -
Flags: approval-mozilla-beta?
Comment 10•6 years ago
|
||
Comment on attachment 8944680 [details] Bug 1432045 - Maintain consistent alignment in case of font fallback for non-Latin characters. Simple fix, taking for 59b8.
Attachment #8944680 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 11•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/a2cd586a63ec
Updated•6 years ago
|
QA Whiteboard: [good first verify]
Comment 13•4 years ago
|
||
Bugbug thinks this bug is a regression, but please revert this change in case of error.
Keywords: regression
You need to log in
before you can comment on or make changes to this bug.
Description
•