Closed
Bug 1399500
Opened 7 years ago
Closed 7 years ago
[Windows 7 & 8] Default, Light and Dark Themes should be structurally consistent
Categories
(Firefox :: Theme, defect, P1)
Tracking
()
VERIFIED
FIXED
Firefox 58
People
(Reporter: shorlander, Assigned: dao)
References
(Blocks 1 open bug)
Details
(Whiteboard: [reserve-photon-visual])
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
johannh
:
review+
ritu
:
approval-mozilla-beta+
|
Details |
The Default, Light and Dark themes should be structurally consistent. The only difference should be in color value. Currently there is some shifting of elements between the default and the light / dark. http://design.firefox.com/people/shorlander/photon/Mockups/windows-7.html
Updated•7 years ago
|
Status: NEW → ASSIGNED
Iteration: --- → 57.3 - Sep 19
Flags: qe-verify?
Whiteboard: [photon-visual] → [reserve-photon-visual]
Comment 1•7 years ago
|
||
This will need some more thorough investigation, but I don't think I can prioritize this over other stuff that's happening right now, so moving back to reserve backlog.
Assignee: jhofmann → nobody
Status: ASSIGNED → NEW
Iteration: 57.3 - Sep 19 → ---
Flags: qe-verify? → qe-verify+
Priority: P1 → P3
Updated•7 years ago
|
QA Contact: ovidiu.boca
Assignee | ||
Updated•7 years ago
|
Priority: P3 → P4
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Priority: P4 → P1
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8915049 -
Flags: review?(jhofmann)
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8915049 [details] Bug 1399500 - Light and Dark themes should use the same toolbar borders as the default theme. https://reviewboard.mozilla.org/r/186316/#review192286 ::: browser/themes/windows/browser.css:82 (Diff revision 3) > display: none; > } > > :root[sizemode="normal"][chromehidden~="menubar"] #toolbar-menubar ~ #TabsToolbar, > :root[sizemode="normal"] #toolbar-menubar[autohide="true"][inactive] ~ #TabsToolbar { > - margin-top: var(--space-above-tabbar); > + padding-top: var(--space-above-tabbar); Is this change required to solve the bug? I can't perceive differences in vertical spacing on Windows 7 between the themes. This leads (afaict) to a regression on Mac where the tab line is 1px too thin. I haven't really figured out why yet, but I'd prefer to avoid doing that (also to avoid complexity for uplift) if this was just an optimization that we can simply drop from the patch.
Attachment #8915049 -
Flags: review?(jhofmann) → review-
Comment 6•7 years ago
|
||
(In reply to Johann Hofmann [:johannh] from comment #5) > Comment on attachment 8915049 [details] > Bug 1399500 - Light and Dark themes should use the same toolbar borders as > the default theme. > > https://reviewboard.mozilla.org/r/186316/#review192286 > > ::: browser/themes/windows/browser.css:82 > (Diff revision 3) > > display: none; > > } > > > > :root[sizemode="normal"][chromehidden~="menubar"] #toolbar-menubar ~ #TabsToolbar, > > :root[sizemode="normal"] #toolbar-menubar[autohide="true"][inactive] ~ #TabsToolbar { > > - margin-top: var(--space-above-tabbar); > > + padding-top: var(--space-above-tabbar); > > Is this change required to solve the bug? I can't perceive differences in > vertical spacing on Windows 7 between the themes. > > This leads (afaict) to a regression on Mac where the tab line is 1px too > thin. I haven't really figured out why yet, but I'd prefer to avoid doing > that (also to avoid complexity for uplift) if this was just an optimization > that we can simply drop from the patch. To be clear, the problem rather lies in the adjusted calculation in tabsintitlebar.js, not in the Windows-specific browser.css file.
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Johann Hofmann [:johannh] from comment #5) > Comment on attachment 8915049 [details] > Bug 1399500 - Light and Dark themes should use the same toolbar borders as > the default theme. > > https://reviewboard.mozilla.org/r/186316/#review192286 > > ::: browser/themes/windows/browser.css:82 > (Diff revision 3) > > display: none; > > } > > > > :root[sizemode="normal"][chromehidden~="menubar"] #toolbar-menubar ~ #TabsToolbar, > > :root[sizemode="normal"] #toolbar-menubar[autohide="true"][inactive] ~ #TabsToolbar { > > - margin-top: var(--space-above-tabbar); > > + padding-top: var(--space-above-tabbar); > > Is this change required to solve the bug? Yes. Margin is outside the toolbar's border box, padding is inside.
Comment hidden (mozreview-request) |
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8915049 [details] Bug 1399500 - Light and Dark themes should use the same toolbar borders as the default theme. https://reviewboard.mozilla.org/r/186316/#review192306 Great, thank you!
Attachment #8915049 -
Flags: review?(jhofmann) → review+
Comment 10•7 years ago
|
||
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/59aef3210d5e Light and Dark themes should use the same toolbar borders as the default theme. r=johannh
![]() |
||
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/59aef3210d5e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8915049 [details] Bug 1399500 - Light and Dark themes should use the same toolbar borders as the default theme. Approval Request Comment [Feature/Bug causing the regression]: photon polish [User impact if declined]: toolbars lack borders with the Light and Dark themes in non-maximized windows in Windows 7 and 8 [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: not yet [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?]: basically just moving a few CSS borders around, and replacing the tabs toolbar's margin with padding on Windows (we already did this before on Mac) [String changes made/needed]: /
Attachment #8915049 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•7 years ago
|
OS: All → Windows
Summary: [Windows 7] Default, Light and Dark Themes should be structurally consistent → [Windows 7 & 8] Default, Light and Dark Themes should be structurally consistent
Comment on attachment 8915049 [details] Bug 1399500 - Light and Dark themes should use the same toolbar borders as the default theme. Photon polish, Beta57+
Attachment #8915049 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 14•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/55322dd4aa31
Comment 15•7 years ago
|
||
I verified this issue using Nightly 58.0a1 and Firefox Beta 57.0b9 on Windows 7 x64 and Windows 8 x64 with Build ID 20171016185129. I can confirm the fix.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•