Closed
Bug 1104076
Opened 8 years ago
Closed 8 years ago
Items in subdialogs dropdowns are huge
Categories
(Firefox :: Settings UI, defect)
Firefox
Settings UI
Tracking
()
VERIFIED
FIXED
Firefox 36
People
(Reporter: u428464, Assigned: Paenglab)
References
Details
(Whiteboard: regression)
Attachments
(3 files, 5 obsolete files)
When I want to select a font in the "advanced" section of "content" the fonts names in the dropdown are huge, much bigger than the current font. I don't see this issue in other preferences dropdowns.
Seems to be related to all in-content subdialogs since the "add languages" dropdown is also affected.
Summary: Fonts names in content/advanced are huge → Items in subdialogs dropdowns are huge
Blocks: ship-incontent-prefs
Updated•8 years ago
|
Updated•8 years ago
|
OS: Windows 7 → All
Assignee | ||
Comment 3•8 years ago
|
||
Reduce the font-size.
Comment 4•8 years ago
|
||
Comment on attachment 8527925 [details] [diff] [review] bug1104076.patch The height shouldn't be hardcoded to 25px. Also, I think 1em (rather than 1rem) should do the right thing here. If it doesn't I'd like to know why.
Attachment #8527925 -
Flags: review?(dao) → review-
Assignee | ||
Comment 5•8 years ago
|
||
I use now height: 1.5em;. This gives on my PC 24px.
Attachment #8527925 -
Attachment is obsolete: true
Attachment #8528433 -
Flags: review?(dao)
Comment 6•8 years ago
|
||
Comment on attachment 8528433 [details] [diff] [review] bug1104076.patch >+menulist > menupopup menu, >+menulist > menupopup menuitem { Please use the child selector, e.g. menulist > menupopup > menuitem should work, I think. I'm not quite sure why this selector is targeting menu elements too. Do we support nested popups for menulists? >+ height: 1.5em; Why do you need to specify a height at all? It's not clear to me what you're trying to fix here.
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #6) > Comment on attachment 8528433 [details] [diff] [review] > bug1104076.patch > > >+menulist > menupopup menu, > >+menulist > menupopup menuitem { > > Please use the child selector, e.g. menulist > menupopup > menuitem should > work, I think. > > I'm not quite sure why this selector is targeting menu elements too. Do we > support nested popups for menulists? Yes they are all simple menulists, so menulist > menupopup > menuitem is enough. > >+ height: 1.5em; > > Why do you need to specify a height at all? It's not clear to me what you're > trying to fix here. Because http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/in-content/common.inc.css#296 sets 30px and this is to tall with the smaller font-size.
Attachment #8528433 -
Attachment is obsolete: true
Attachment #8528433 -
Flags: review?(dao)
Attachment #8528507 -
Flags: review?(dao)
Comment 8•8 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #7) > > >+ height: 1.5em; > > > > Why do you need to specify a height at all? It's not clear to me what you're > > trying to fix here. > > Because > http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/in- > content/common.inc.css#296 sets 30px and this is to tall with the smaller > font-size. Seems like we should remove that (maybe replace it with something saner if needed). In fact, isn't font-size: 1.25rem in that rule what's causing this bug in the first place? The hardcoded line-height looks wrong too, by the way.
Assignee | ||
Comment 9•8 years ago
|
||
I changed the height: 30px; to 2em. The taller font-size in the normal in-content pages are from style specs. The specs also want for in-content dialogs smaller fonts and this needs the code for smaller fonts in dialog.css. I also removed the line-height.
Attachment #8528507 -
Attachment is obsolete: true
Attachment #8528507 -
Flags: review?(dao)
Attachment #8528530 -
Flags: review?(dao)
Comment 10•8 years ago
|
||
Comment on attachment 8528530 [details] [diff] [review] bug1104076.patch >--- a/toolkit/themes/shared/in-content/common.inc.css >+++ b/toolkit/themes/shared/in-content/common.inc.css >@@ -287,18 +287,17 @@ xul|button[type="menu"] > xul|menupopup > } > > xul|menulist > xul|menupopup xul|menu, > xul|menulist > xul|menupopup xul|menuitem, > xul|button[type="menu"] > xul|menupopup xul|menu, > xul|button[type="menu"] > xul|menupopup xul|menuitem { > -moz-appearance: none; > font-size: 1.25rem; >- line-height: 22px; >- height: 30px; >+ height: 2em; > color: #333; > -moz-padding-start: 10px; > -moz-padding-end: 30px; > } Seems like font-size: 1em should be used here, and then you can stop setting it in dialog.inc.css. We also shouldn't set a height at all but instead use vertical padding.
Attachment #8528530 -
Flags: review?(dao) → review-
Assignee | ||
Comment 11•8 years ago
|
||
Yes, 1em does the thing. I'm now using padding instead of height.
Attachment #8528530 -
Attachment is obsolete: true
Attachment #8528551 -
Flags: review?(dao)
Comment 12•8 years ago
|
||
Would it be possible to use a relative unit for the padding (i.e. em) so that you don't have to override the value in dialog.inc.css?
Reporter | ||
Comment 13•8 years ago
|
||
I don't know if the patch also covers this : but the "sort categorie" fonts in subdialogs are also too big.
Assignee | ||
Comment 14•8 years ago
|
||
Patch with relative units padding.
Attachment #8528551 -
Attachment is obsolete: true
Attachment #8528551 -
Flags: review?(dao)
Attachment #8528580 -
Flags: review?(dao)
Assignee | ||
Comment 15•8 years ago
|
||
(In reply to Guillaume C. [:ge3k0s] from comment #13) > I don't know if the patch also covers this : but the "sort categorie" fonts > in subdialogs are also too big. Do you mean the treecols? If yes, are you using a Nightly or build with bug 1087618 applied? If the latter I'm seeing this only with the patch of bug 1087618 applied.
Reporter | ||
Comment 16•8 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #15) > (In reply to Guillaume C. [:ge3k0s] from comment #13) > > I don't know if the patch also covers this : but the "sort categorie" fonts > > in subdialogs are also too big. > > Do you mean the treecols? If yes, are you using a Nightly or build with bug > 1087618 applied? If the latter I'm seeing this only with the patch of bug > 1087618 applied. Just to be sure as an example are treecols used to sort by name or by password (when using the passwords subdialogs) ? If so yes that's what I'm talking about. I'm using regular Nightly without the patch you mentioned.
Assignee | ||
Comment 17•8 years ago
|
||
Yes, that should be the treecols. I'm not seeing this issue. Maybe file a new bug for this.
Reporter | ||
Comment 18•8 years ago
|
||
Just to be sure are the treecols fonts supposed to be this size ?
Comment 19•8 years ago
|
||
Comment on attachment 8528580 [details] [diff] [review] bug1104076.patch Thanks!
Attachment #8528580 -
Flags: review?(dao) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 20•8 years ago
|
||
(In reply to Guillaume C. [:ge3k0s] from comment #18) > Created attachment 8529030 [details] > Treecols font > > Just to be sure are the treecols fonts supposed to be this size ? Please file a bug for this treecols.
Reporter | ||
Comment 21•8 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #20) > (In reply to Guillaume C. [:ge3k0s] from comment #18) > > Created attachment 8529030 [details] > > Treecols font > > > > Just to be sure are the treecols fonts supposed to be this size ? > > Please file a bug for this treecols. Filed bug 1105254.
Comment 22•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/0323dcdc16f9
Keywords: checkin-needed
Comment 23•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0323dcdc16f9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Comment 25•8 years ago
|
||
Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using latest Nightly 37.0a1 (buildID: 20141204030201) and latest Aurora 36.0a2 (buildID: 20141204004001).
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•