Closed Bug 1399747 Opened 2 years ago Closed 2 years ago

The font-size from Theme and UI Density in not consistent (Mac OS)

Categories

(Firefox :: Theme, defect, P1, minor)

57 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 58
Tracking Status
firefox56 --- unaffected
firefox57 --- verified
firefox58 --- verified

People

(Reporter: Ovidiu, Assigned: nhnt11)

References

(Blocks 1 open bug)

Details

(Whiteboard: [reserve-photon-visual])

Attachments

(2 files)

Attached image the font size.png
This is a follow up from https://bugzilla.mozilla.org/show_bug.cgi?id=1380955#c14

[Affected versions]: Nightly 57.0a1(2017-09-13)

[Affected platforms]: Tested on Mac OS 

[Steps to reproduce]:

1. Go to Customize
2. Compare the font-size from Themes and from Density

[Expected result]:

The font-size should be the same. 

[Actual result]:

The font size is not the same, for example, the "N" from Normal has 11px and the "D" from Default has 10px have different px size. See the attached print screen.
Flags: qe-verify+
Priority: -- → P4
Whiteboard: [reserve-photon-visual]
QA Contact: ovidiu.boca
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Iteration: --- → 57.3 - Sep 19
Priority: P4 → P1
There's already a rule[1] that was likely intended for this, but it's applied to the panel and intended to be inherited by its children. The problem is that menuitem elements have `font: -moz-pull-down-menu;` set on them[2], which overrules [1].

I'd be interested to investigate whether we can remove that rule from menu.css (do we use any menuitem elements that rely on that rule?) but that's out of the scope of this bug.

Johann, I want to keep the density menu selector for [1] for correctness even though it's currently not being used, let me know what you think.

[1] https://searchfox.org/mozilla-central/rev/2ef8bd8a46a02c68ddbb1d5f25fa254dd7be1fbd/browser/themes/shared/customizableui/customizeMode.inc.css#303
[2] https://searchfox.org/mozilla-central/rev/2ef8bd8a46a02c68ddbb1d5f25fa254dd7be1fbd/toolkit/themes/osx/global/menu.css#13
Comment on attachment 8910562 [details]
Bug 1399747 - Density panel menuitems should use `font: menu` to be consistent with other panels.

https://reviewboard.mozilla.org/r/182006/#review187484

Hm I think I'd be fine with either removing or leaving the unused rule. If you prefer to leave it, that's ok.
Attachment #8910562 - Flags: review?(jhofmann) → review+
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e6a2ee5c3532
Density panel menuitems should use `font: menu` to be consistent with other panels. r=johannh
Iteration: 57.3 - Sep 19 → ---
https://hg.mozilla.org/mozilla-central/rev/e6a2ee5c3532
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Build ID: 20170924220116
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:58.0) Gecko/20100101 Firefox/58.0

Verified as fixed on Firefox Nightly 58.0a1 on Mac OS X 10.12.
Status: RESOLVED → VERIFIED
Comment on attachment 8910562 [details]
Bug 1399747 - Density panel menuitems should use `font: menu` to be consistent with other panels.

Approval Request Comment
[Feature/Bug causing the regression]: photon-visual polish
[User impact if declined]: inconsistent font size in two different menus in customize mode
[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]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: minor visual change
[String changes made/needed]:
Attachment #8910562 - Flags: approval-mozilla-beta?
Comment on attachment 8910562 [details]
Bug 1399747 - Density panel menuitems should use `font: menu` to be consistent with other panels.

Polish photon, taking it.
Should be in 57b4. GTB later today.
Attachment #8910562 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Build ID: 20170928180207
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:57.0) Gecko/20100101 Firefox/57.0

Verified as fixed on Firefox Beta 57.0b5 on Mac OS X 10.12.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.