Closed Bug 1405542 Opened 2 years ago Closed 2 years ago

Containers tab underline is too hard to see at 150% DPI

Categories

(Firefox :: Theme, defect, P1)

58 Branch
x86
Windows 10
defect

Tracking

()

VERIFIED FIXED
Firefox 58
Tracking Status
firefox58 --- verified
firefox59 --- verified

People

(Reporter: jkt, Assigned: dao)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [reserve-photon-visual])

Attachments

(4 files)

Specifically in dark theme this is noticeable.

The position of the underline was approved in the previous bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1387117

I think it is wrong by one pixel but perhaps we should look at if there is anything else we can do.

The current selected tab shows 1px under and 1px over the line. Other tabs just show the one above.

A vast improvement is had just by adding:

.tab-bottom-line {
  margin-block-end: 1px;
}

I have attached a preview of the comparison.
Is the proposed solution ok for you both?
Flags: needinfo?(mochen)
Flags: needinfo?(dao+bmo)
Yes, I believe the treatment can solve the problem. However, it still needs the 1px radius according to the measurement here:https://www.dropbox.com/s/y9j5pvbzne0xhzw/Container-tabbar.png?dl=0. Thanks.
Flags: needinfo?(mochen)
Why would you let the underline float rather than increasing it's height?
Flags: needinfo?(dao+bmo)
Attached image tab-highlight-issue.png
So there is half pixel of drop shadow that makes the non selected tab look harder to see.

The actual problem is the tab line sits one pixel below the non selected line. This is only easily noticeable when you zoom in and draw a line like attached.

When the tab is selected line shows 2px. Making this indicator 3px tall when selected looks just as broken when selected but fixes it in non selected.

Last time I looked into this I think the non selected tab masks the tab by 1px and it looked like that might not be possible to fix that way.
Flags: needinfo?(dao+bmo)
(In reply to Jonathan Kingston [:jkt] from comment #5)
> Created attachment 8915574 [details]
> tab-highlight-issue.png

> The actual problem is the tab line sits one pixel below the non selected
> line. This is only easily noticeable when you zoom in and draw a line like
> attached.

Is this on Windows 10? Do you see it elsewhere? I can't seem to reproduce this on Linux.
Flags: needinfo?(dao+bmo)
Yeah Windows 10, Mac appears to be the same or similar at least (I don't really have a decent mac setup atm).
Flags: needinfo?(dao+bmo)
What's your DPI setting? Can you reproduce at 100% or 200%?
Flags: needinfo?(dao+bmo)
My scaling was 150%, looks ok at 100%.
I'm not sure how this could change it per tab though. The Mac was connected to a big screen but I was checking it on the laptop itself.
The latest update for tab size now messes with the padding either side too :/ (that should likely be a separate bug).
Priority: -- → P3
Assignee: jkt → dao+bmo
Blocks: photon-tabs
Status: NEW → ASSIGNED
Component: Tabbed Browser → Theme
Flags: qe-verify+
Priority: P3 → P1
Whiteboard: [reserve-photon-visual]
Blocks: 1395436
Blocks: 1398395
Summary: Containers underline is too hard to see in Photon → Containers tab underline is too hard to see at 150% DPI
Comment on attachment 8920169 [details]
Bug 1405542 - Replace nav-bar top border and the tabs toolbar's negative margin with a box-shadow to avoid running into bug 477157.

https://reviewboard.mozilla.org/r/191210/#review197100

I tested this patch on different DPI settings and it works okay for me.
Attachment #8920169 - Flags: review?(nhnt11) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f4343e201285
Replace nav-bar top border and the tabs toolbar's negative margin with a box-shadow to avoid running into bug 477157. r=nhnt11
Backed out for failing in OS X (browser chrome test) at browser/base/content/test/performance/browser_windowopen_reflows.js:

https://hg.mozilla.org/integration/autoland/rev/e1e574000af4d1d989d2ca3435dc7e7898691893

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f4343e2012854ea6806047359b7e39c716872359&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=138867588&repo=autoland

05:23:35     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_windowopen_reflows.js | Unused expected reflow: [
05:23:35     INFO - 	"verticalMargins@chrome://browser/content/browser-tabsintitlebar.js",
05:23:35     INFO - 	"_update@chrome://browser/content/browser-tabsintitlebar.js",
05:23:35     INFO - 	"init@chrome://browser/content/browser-tabsintitlebar.js",
05:23:35     INFO - 	"handleEvent@chrome://browser/content/tabbrowser.xml"
05:23:35     INFO - ]
Flags: needinfo?(dao+bmo)
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f5121cba7d7b
Replace nav-bar top border and the tabs toolbar's negative margin with a box-shadow to avoid running into bug 477157. r=nhnt11
https://hg.mozilla.org/mozilla-central/rev/f5121cba7d7b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Depends on: 1411642
Depends on: 1411648
Screenshots:

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=a124f4901430f6db74cfc7fe3b07957a1c691b40&newProject=mozilla-central&newRev=252a8528c5ab4affc2742e7ccfe4ef4aa5ee42f8

Shows bug 1411648 quite clearly, but it also shows that the window control buttons on OSX seem to have moved down by 1px, which is good, I think, because they're exactly centered now.
I tested this issue on Mac OS X 10.10 and Windows 10 with FF Nightly 58.0a1(2017-10-30) and please see the attached file, the line has 2 px.  Jonathan do you have the same result with the latest Nightly?
Flags: needinfo?(jkt)
Depends on: 1412557
No longer blocks: 1398395
Yup looks good for me on Linux which I am on at the moment. I think it was good last week on Windows too.
Flags: needinfo?(jkt)
Thanks Jonathan for your help, I will mark this as verified as fixed based on comments 19 and 21.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.