Closed
Bug 1162441
Opened 10 years ago
Closed 8 years ago
[HiDPI] Separator between toolbars and content is sometimes too thick at 125% scale
Categories
(Firefox :: Theme, defect, P3)
Firefox
Theme
Tracking
()
RESOLVED
FIXED
Firefox 51
Tracking | Status | |
---|---|---|
firefox39 | --- | unaffected |
firefox40 | --- | affected |
firefox51 | --- | fixed |
People
(Reporter: aryx, Assigned: dao)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(2 files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
jaws
:
review+
|
Details | Diff | Splinter Review |
Firefox Nightly 20150506 on Windows 8.1
The line which separates the navbox from the content has twice the size it has on Aurora. Inspecting the ::after pseudo element with browser toolbox says the height is 1 px, but measuring it in the screenshot shows 2px.
Comment 1•10 years ago
|
||
Are you using a hidpi screen and/or windows setup?
Flags: needinfo?(archaeopteryx)
Reporter | ||
Comment 2•10 years ago
|
||
The Windows 8.1 is set to scale fonts to 125% by default. Toggling hidpi with https://addons.mozilla.org/editors/review/fxdpi doesn't let the thick line shrink.
Flags: needinfo?(archaeopteryx)
Comment 3•10 years ago
|
||
(In reply to Archaeopteryx [:aryx] from comment #2)
> The Windows 8.1 is set to scale fonts to 125% by default.
This depends on your screen/computer, I think. Mine defaulted to 100%, I think? That's certainly what it's currently set at...
Do you have time to find a regression window? :-)
Flags: needinfo?(archaeopteryx)
Keywords: regression,
regressionwindow-wanted
Reporter | ||
Comment 4•10 years ago
|
||
This is a regression from http://hg.mozilla.org/mozilla-central/rev/87f15ce8266d / bug 1147702 and only visible if the menubar is hidden.
Blocks: 1147702
status-firefox39:
--- → unaffected
Flags: needinfo?(archaeopteryx)
Keywords: regressionwindow-wanted
Reporter | ||
Comment 5•10 years ago
|
||
The nav-bar has different heights:
Without bug: 39.6px
With bug: 40px
Dao, do you have any ideas what has changed?
Flags: needinfo?(dao)
Reporter | ||
Comment 6•10 years ago
|
||
This does only occur if there are buttons without the class "badged-button" in the toolbar. E.g. remove all toolbar buttons except Loop and this issue will go away.
(In reply to Sebastian H. [:aryx][:archaeopteryx] from comment #5)
> The nav-bar has different heights:
>
> Without bug: 39.6px
> With bug: 40px
>
> Dao, do you have any ideas what has changed?
For me it's 40.8px on HiDPI and 41px on normal DPI (100%) [Win7_64, Nightly 45 (2015-10-31), 32bit]
I think the answer is bug 477157 which breaks borders on zoom / transform:scale() .
Note that even users with DPI = 100% can _test_ this bug by setting layout.css.devPixelsPerPx -> 1.25
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] from comment #5)
> The nav-bar has different heights:
>
> Without bug: 39.6px
> With bug: 40px
>
> Dao, do you have any ideas what has changed?
no idea
Flags: needinfo?(dao+bmo)
Assignee | ||
Comment 9•8 years ago
|
||
(In reply to arni2033 from comment #7)
> I think the answer is bug 477157 which breaks borders on zoom /
> transform:scale() .
Sounds about right. So we could fix this bug by setting a border on #navigator-toolbox::after instead of using background. Would you like to write that patch?
Flags: needinfo?(arni2033)
Priority: -- → P3
Comment 10•8 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #9)
> Sounds about right. So we could fix this bug by setting a border on #navigator-toolbox::after
> instead of using background. Would you like to write that patch?
Sorry for long response. No, because I'm too busy saving bugs locally.
Your solution wasn't immediately obvious to me, so I'll leave this note anyway:
Comment 7 only explains why toolbox has fractional height in this particular case, but it's not the
root of the problem (see [1]). So I didn't immediately understand that bug477157 is a way to _fix_
this, and solution will work as long as bug477157 is present. (Does it mean that 477157 is WONTFIX?..)
Testcases: bug 1224732 comment 5, url [1], url [2]
> [1] data:text/html,<!DOCTYPE html><body id="B" onload="S='';for(i=0;i<10;i++)S+='<div style=\'margin:0px 2px '+i+'px 0px;width:15px;height:1px;background:black;display:inline-block;\'></div>';for(i=0;i<10;i++)S+='<div style=\'margin:0px 0px 2px '+i+'px;height:15px;width:1px;background:black;\'></div>';B.innerHTML=S+'<title>Even elements w/ integer position have different width on zoom'">
> [2] data:text/html,<!DOCTYPE html><body id="B" onload="S='';for(i=0;i<10;i+=(i<1)?0.1:1)S+='<div style=\'margin:0px 2px '+i+'px 0px;width:15px;border-bottom:1px solid black;display:inline-block;\'></div>';for(i=0;i<10;i+=(i<1)?0.1:1)S+='<div style=\'margin:0px 0px 2px '+i+'px;height:15px;border-left:1px solid black;\'></div>';B.innerHTML=S+'<title>Elements have equal width on any zoom level'">
Flags: needinfo?(arni2033)
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to arni2033 from comment #10)
> Your solution wasn't immediately obvious to me, so I'll leave this note
> anyway:
> Comment 7 only explains why toolbox has fractional height in this particular
> case, but it's not the
> root of the problem (see [1]). So I didn't immediately understand that
> bug477157 is a way to _fix_
> this, and solution will work as long as bug477157 is present. (Does it mean
> that 477157 is WONTFIX?..)
Hm, I expected bug 477157 would (if fixed) change how backgrounds are rendered, not borders. Maybe it's the other way around.
Assignee | ||
Updated•8 years ago
|
Summary: separator line (pseude ::after element) between nav-box (tabbox with tabs, bookmarks toolbar) and content has increased height (2px instead of 1) → separator line (pseudo ::after element) between nav-box (tabbox with tabs, bookmarks toolbar) and content has increased height (2px instead of 1)
Assignee | ||
Comment 12•8 years ago
|
||
Oh wait... does anyone actually still see this bug? I can't seem to reproduce it with latest Nightly on Windows 10 @ 125%.
Flags: needinfo?(aryx.bugmail)
Flags: needinfo?(arni2033)
Reporter | ||
Comment 13•8 years ago
|
||
Cant reproduce anymore with latest Nightly on the same machine.
Flags: needinfo?(aryx.bugmail)
Comment 14•8 years ago
|
||
>>> My Info: Win7_64, Nightly 51, 32bit, ID 20160902030222 (2016-09-02)
Can reproduce on a clear profile (launched via Mozregression GUI).
In some configurations of toolbar buttons and toolbars this bug manifests, in others it does not.
BTW, sorry for generating all the spam, but I actually think solution in comment 9 is good in long term, because (I believe) bug 477157 stays for a long time as is.
Flags: needinfo?(arni2033)
Assignee | ||
Comment 15•8 years ago
|
||
I can't reproduce this in a new profile either :/
But if you can reproduce this, maybe you could also check if my patch helps: https://archive.mozilla.org/pub/firefox/try-builds/dgottwald@mozilla.com-b6022e130526b4f929bec09a7507eb455516dd91/try-win32/
Reporter | ||
Comment 16•8 years ago
|
||
So I eventually can reproduce with a totally new profile, no idea what makes the difference to my testing stub. Your patch fixes the issue for me.
Assignee | ||
Comment 17•8 years ago
|
||
AFAIK there's no reason to prefer background over border here... so I think this is a reasonable change even if bug 477157 may at some point make background scale like border (making this fix unnecessary as far as this bug is concerned) or vice versa (making this fix ineffective).
Assignee | ||
Updated•8 years ago
|
Summary: separator line (pseudo ::after element) between nav-box (tabbox with tabs, bookmarks toolbar) and content has increased height (2px instead of 1) → [HiDPI] Separator between toolbars and content is sometimes too thick at 125% scale
Comment 18•8 years ago
|
||
Comment on attachment 8787769 [details] [diff] [review]
patch
Review of attachment 8787769 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/themes/osx/browser.css
@@ +67,5 @@
> @media (-moz-mac-yosemite-theme) {
> #navigator-toolbox:-moz-window-inactive::after {
> + border-top-style: none;
> + border-bottom-color: hsla(0,0%,0%,.1);
> + margin-top: -1px;
Can you explain why the margin-top should change here? And why the border-top isn't `1px solid hsla(0,0%,100%,0)`? Previously only the background-image changed and the stops of the linear-gradient were the same distance.
Attachment #8787769 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 19•8 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #18)
> Comment on attachment 8787769 [details] [diff] [review]
> patch
>
> Review of attachment 8787769 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: browser/themes/osx/browser.css
> @@ +67,5 @@
> > @media (-moz-mac-yosemite-theme) {
> > #navigator-toolbox:-moz-window-inactive::after {
> > + border-top-style: none;
> > + border-bottom-color: hsla(0,0%,0%,.1);
> > + margin-top: -1px;
>
> Can you explain why the margin-top should change here? And why the
> border-top isn't `1px solid hsla(0,0%,100%,0)`? Previously only the
> background-image changed and the stops of the linear-gradient were the same
> distance.
Because the negative margin-top matches the combined border width and hsla(0,0%,100%,0) is completely transparent so there's no point in having that border.
Comment 20•8 years ago
|
||
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd747d43e7c7
Use CSS border instead of background for the separator between toolbars and content for more predictable high-DPI behavior. r=jaws
Comment 21•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
You need to log in
before you can comment on or make changes to this bug.
Description
•