[HiDPI] Separator between toolbars and content is sometimes too thick at 125% scale

RESOLVED FIXED in Firefox 51

Status

()

Firefox
Theme
P3
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: aryx, Assigned: dao)

Tracking

(Blocks: 1 bug, {regression})

Trunk
Firefox 51
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox39 unaffected, firefox40 affected, firefox51 fixed)

Details

Attachments

(2 attachments)

Created attachment 8602617 [details]
screenshot

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

3 years ago
Are you using a hidpi screen and/or windows setup?
Flags: needinfo?(archaeopteryx)
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

3 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
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
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)
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.

Comment 7

3 years ago
(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
See Also: → bug 477157
(Assignee)

Comment 8

2 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

2 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

2 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

2 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

2 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

2 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)
Cant reproduce anymore with latest Nightly on the same machine.
Flags: needinfo?(aryx.bugmail)

Comment 14

2 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

2 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/
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

2 years ago
Created attachment 8787769 [details] [diff] [review]
patch

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: nobody → dao+bmo
Status: NEW → ASSIGNED
Attachment #8787769 - Flags: review?(jaws)
(Assignee)

Updated

2 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
(Assignee)

Updated

2 years ago
Blocks: 820679
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

2 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

2 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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fd747d43e7c7
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51

Updated

2 years ago
See Also: → bug 1327324
You need to log in before you can comment on or make changes to this bug.