Closed
Bug 1192214
Opened 9 years ago
Closed 9 years ago
[Control Center] Icons don't align with section headlines
Categories
(Firefox :: General, defect, P1)
Firefox
General
Tracking
()
People
(Reporter: ttaubert, Assigned: bgrins)
References
Details
(Whiteboard: [fxprivacy])
Attachments
(4 files)
Ash sent me a few screenshots a week or two ago that show that the top line of the headline is not aligned with the top border of the icon to the left. I tried fixing that but wasn't very successful, there seems to be a "padding" that the font rendering creates and as that's flexible depending on the font and size I wouldn't know how to fix this properly.
Flags: firefox-backlog?
Updated•9 years ago
|
Updated•9 years ago
|
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Iteration: --- → 43.1 - Aug 24
Flags: qe-verify?
Priority: P2 → P1
Assignee | ||
Comment 1•9 years ago
|
||
Ash, can you attach the screenshots showing this misalignment in the Control Center?
Flags: needinfo?(agrigas)
Assignee | ||
Comment 2•9 years ago
|
||
I think I figured this out. It looks like the logo is currently just a *touch* too high (as you can see in this screenshot where i've replaced the background image of the section with gradients and drawn a line across the TP section
Flags: needinfo?(agrigas)
Assignee | ||
Comment 3•9 years ago
|
||
Same thing, but with my patch applied
Assignee | ||
Comment 4•9 years ago
|
||
I realized that my patch makes the header container top-aligned with the image, but the text still doesn't have a sharp top alignment with the image. Is this what we want? (see attached screenshot)
Note that if we want the top of the text to align with the top of the image, it won't always happen in the case where all of the text is lower cased (like about:preferences)
Flags: needinfo?(agrigas)
Assignee | ||
Comment 5•9 years ago
|
||
I guess qe-verify+ but it's going to be very subtle.
Flags: qe-verify? → qe-verify+
Comment 6•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #4)
> Created attachment 8646625 [details]
> new-no-background-color.png
>
> I realized that my patch makes the header container top-aligned with the
> image, but the text still doesn't have a sharp top alignment with the image.
> Is this what we want? (see attached screenshot)
>
> Note that if we want the top of the text to align with the top of the image,
> it won't always happen in the case where all of the text is lower cased
> (like about:preferences)
Can we at least have the ascender top aline (the top of the letter). Its ok if that breaks when its all lower-case or has no ascenders.
https://www.dropbox.com/s/fr3vh38zzpuhjgc/Screenshot%202015-08-11%2015.11.21.png?dl=0
Flags: needinfo?(agrigas)
Updated•9 years ago
|
QA Contact: mwobensmith
Assignee | ||
Comment 7•9 years ago
|
||
Looked further into it, it appears that there is some line-height that is set when <label value="foo" /> is used that isn't accessible via CSS on the label selector. But if you do <label>foo</label> you can set line-height: 1 which aligns it properly. Unfortunately I'm not sure if it'll be practical to switch to this, since setting it to use textContent doesn't seem to crop long strings correctly.
Assignee | ||
Comment 8•9 years ago
|
||
So I spent some time debugging inside nsTextBoxFrame (which is the frame used when setting value="" as opposed to textContent). As best as I can tell, I think GetTextSize [0] is somehow not taking into account the line height of the element.
I'm going to upload a patch that makes things better across systems by just tweaking the existing padding / margin, but I think it's not going to be possible to get this pixel-perfect across all default fonts unless if we come up with a fix in the XUL layout. I don't think it's worth spending more time on this, though.
[0]: https://dxr.mozilla.org/mozilla-central/source/layout/xul/nsTextBoxFrame.cpp?#1017
Assignee | ||
Comment 9•9 years ago
|
||
With the default fonts on my test systems, Linux seems to have the smallest gap (align the text higher), OSX in the middle, and Windows has the biggest gap (aligns the text lower)
Assignee | ||
Comment 10•9 years ago
|
||
Bug 1192214 - Use matching alignment for section headers and icons in control center;r=ttaubert
Attachment #8647263 -
Flags: review?(ttaubert)
Reporter | ||
Comment 11•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #7)
> Looked further into it, it appears that there is some line-height that is
> set when <label value="foo" /> is used that isn't accessible via CSS on the
> label selector. But if you do <label>foo</label> you can set line-height: 1
> which aligns it properly. Unfortunately I'm not sure if it'll be practical
> to switch to this, since setting it to use textContent doesn't seem to crop
> long strings correctly.
That's a great find, it seems like that looks a lot better than what we have now but we can't crop strings properly. I hate XUL, especially in combination with text and overflow :(
Reporter | ||
Comment 12•9 years ago
|
||
Comment on attachment 8647263 [details]
MozReview Request: Bug 1192214 - Use matching alignment for section headers and icons in control center;r=ttaubert
https://reviewboard.mozilla.org/r/15811/#review14277
Seems reasonable. I don't think we can achieve anything better with XUL so we have to optimize for the default case.
Attachment #8647263 -
Flags: review?(ttaubert) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 13•9 years ago
|
||
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Comment 15•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #9)
> With the default fonts on my test systems, Linux seems to have the smallest
> gap (align the text higher), OSX in the middle, and Windows has the biggest
> gap (aligns the text lower)
Verified fixed 43.0a1 (2015-08-24) Win7, OS X 10.10, Ubuntu 14.04
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 16•9 years ago
|
||
Comment on attachment 8647263 [details]
MozReview Request: Bug 1192214 - Use matching alignment for section headers and icons in control center;r=ttaubert
Approval Request Comment
[Feature/regressing bug #]: 1170759
[User impact if declined]: Minor visual alignment issue between icon and section headlines in Control Center
[Describe test coverage new/current, TreeHerder]: No tests, changing some CSS positioning
[Risks and why]: Not much, but it could cause some different misalignment depending on default system fonts
[String/UUID change made/needed]:
Attachment #8647263 -
Flags: approval-mozilla-aurora?
Comment 17•9 years ago
|
||
Comment on attachment 8647263 [details]
MozReview Request: Bug 1192214 - Use matching alignment for section headers and icons in control center;r=ttaubert
Polish, taking it.
Attachment #8647263 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 19•9 years ago
|
||
Verified fixed 42.0a2 (2015-09-09) Win7
You need to log in
before you can comment on or make changes to this bug.
Description
•