Closed
Bug 1186244
Opened 9 years ago
Closed 9 years ago
Remove border artifacts between navbar and tabbar in lw-themes on Windows 10
Categories
(Firefox :: Theme, defect, P2)
Tracking
()
VERIFIED
FIXED
Firefox 42
People
(Reporter: ntim, Assigned: ntim)
References
Details
Attachments
(2 files, 1 obsolete file)
3.98 KB,
patch
|
dao
:
review+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
2.97 KB,
image/png
|
Details |
It is used to emulate an aero-ish look for lw-themes. But we don't need it on Windows 10 or even 8.
Assignee | ||
Updated•9 years ago
|
Blocks: windows-10-issues
Updated•9 years ago
|
Assignee | ||
Updated•9 years ago
|
Summary: Remove box-shadow from nav-bar on Windows 10 → Remove border artifacts between navbar and tabbar in lw-themes
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → ntim.bugs
Status: NEW → ASSIGNED
OS: Unspecified → Windows 10
Summary: Remove border artifacts between navbar and tabbar in lw-themes → Remove border artifacts between navbar and tabbar in lw-themes on Windows 10
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8637901 -
Flags: review?(dao)
Comment 2•9 years ago
|
||
Comment on attachment 8637901 [details] [diff] [review]
Patch
>+ /* Setting background-size to "0 0" for the first
>+ background-image to remove the stroke. */
> .tab-background-middle[visuallyselected=true] {
>- /* Setting background-size to "0 0" for the first
>- background-image to remove the stroke. */
> background-size: 0 0, auto 100%, auto 100%;
> }
>+ .tab-background-middle[visuallyselected=true]:-moz-lwtheme {
>+ background-size: 0 0, auto 100%, auto auto;
>+ }
This is getting increasingly ugly. :( Can we avoid repeating the second and third sizes with CSS variables or preprocessing?
What about this code? http://hg.mozilla.org/mozilla-central/annotate/eee2d49d055c/browser/themes/windows/browser.css#l308
Updated•9 years ago
|
Attachment #8637901 -
Flags: review?(dao)
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #2)
> What about this code?
> http://hg.mozilla.org/mozilla-central/annotate/eee2d49d055c/browser/themes/
> windows/browser.css#l308
Just tried removing this code on my VM, and it looks worse : there's a 1px transparent semitransparent border under the selected tab, which looks quite odd.
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8637901 -
Attachment is obsolete: true
Attachment #8639790 -
Flags: review?(dao)
Assignee | ||
Comment 6•9 years ago
|
||
Comment 7•9 years ago
|
||
(In reply to Tim Nguyen [:ntim] from comment #4)
> (In reply to Dão Gottwald [:dao] from comment #2)
> > What about this code?
> > http://hg.mozilla.org/mozilla-central/annotate/eee2d49d055c/browser/themes/
> > windows/browser.css#l308
>
> Just tried removing this code on my VM, and it looks worse : there's a 1px
> transparent semitransparent border under the selected tab, which looks quite
> odd.
That's strange. Where does that border come from? At least we will need to update the comment to reflect what that margin is really needed for.
Assignee | ||
Comment 8•9 years ago
|
||
I suspect this is due to how the tab image is drawn.
Not sure where it comes from precisely though.
Assignee | ||
Updated•9 years ago
|
Priority: -- → P1
Assignee | ||
Comment 10•9 years ago
|
||
[Tracking Requested - why for this release]: Very unpolished look for lightweight themes on Windows 10 : http://i.imgur.com/JbQ5eIV.png
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to Tim Nguyen [:ntim] from comment #10)
> [Tracking Requested - why for this release]: Very unpolished look for
> lightweight themes on Windows 10 : http://i.imgur.com/JbQ5eIV.png
(mainly that top border on the selected tab)
Comment 12•9 years ago
|
||
Tracked for 40, 41, and 42, so that the visual appearance issues for Win10 as noted above are addressed.
Comment 13•9 years ago
|
||
Unfortunately, this is too late for 40.
status-firefox41:
--- → affected
status-firefox42:
--- → affected
Comment 15•9 years ago
|
||
Still pondering about comment 7. It would be nice to really understand what's going on here. I'll try to investigate this a bit when I find the time.
Flags: needinfo?(dao)
Comment 16•9 years ago
|
||
(In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment #8)
> Created attachment 8639804 [details]
> small-white-border-under-tab.PNG
...
> > [Tracking Requested - why for this release]: Very unpolished look for
> > lightweight themes on Windows 10 : http://i.imgur.com/JbQ5eIV.png
We should definately fix this, but the first issue is subtle and we can't seem to reproduce the second (is there some other condition beyond just a LWT?)
Priority: P1 → P2
Comment 17•9 years ago
|
||
(In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment #8)
> Created attachment 8639804 [details]
> small-white-border-under-tab.PNG
>
> I suspect this is due to how the tab image is drawn.
> Not sure where it comes from precisely though.
When just removing the box-shadow and the margin-top on the navbar, I can't actually reproduce this. Were you using non-100% DPI or something?
Arguably though, the tab doesn't really adjoin the nav-bar smoothly - it causes a "bump" where the curves start and end.
Assignee | ||
Comment 18•9 years ago
|
||
(In reply to Justin Dolske [:Dolske] from comment #16)
> (In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment
> #8)
> > Created attachment 8639804 [details]
> > small-white-border-under-tab.PNG
> ...
> > > [Tracking Requested - why for this release]: Very unpolished look for
> > > lightweight themes on Windows 10 : http://i.imgur.com/JbQ5eIV.png
>
> We should definately fix this, but the first issue is subtle and we can't
> seem to reproduce the second (is there some other condition beyond just a
> LWT?)
The small white bottom border appears when the navbar has it's top (negative) margin removed. Dao asked if that margin was useful. It appears to be useful for what Gijs states in comment 17.
(In reply to :Gijs Kruitbosch from comment #17)
> (In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment
> #8)
> > Created attachment 8639804 [details]
> > small-white-border-under-tab.PNG
> >
> > I suspect this is due to how the tab image is drawn.
> > Not sure where it comes from precisely though.
>
> When just removing the box-shadow and the margin-top on the navbar, I can't
> actually reproduce this. Were you using non-100% DPI or something?
>
> Arguably though, the tab doesn't really adjoin the nav-bar smoothly - it
> causes a "bump" where the curves start and end.
My VM is configured on 100% DPI. Although I run this VM from a retina screen (but this doesn't matter since the VM emulates a normal screen).
Comment 19•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #17)
> (In reply to Tim Nguyen [:ntim] (mostly away until 26 August) from comment
> #8)
> > Created attachment 8639804 [details]
> > small-white-border-under-tab.PNG
> >
> > I suspect this is due to how the tab image is drawn.
> > Not sure where it comes from precisely though.
>
> When just removing the box-shadow and the margin-top on the navbar, I can't
> actually reproduce this.
I don't see this either.
> Arguably though, the tab doesn't really adjoin the nav-bar smoothly - it
> causes a "bump" where the curves start and end.
Seems like we should fix that by removing the bottom pixel row of the tab images that was supposed to be moved over the tab bar, but that would probably go beyond the scope of this bug.
Updated•9 years ago
|
Attachment #8639790 -
Flags: review?(dao) → review+
Updated•9 years ago
|
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 20•9 years ago
|
||
Keywords: checkin-needed
Comment 21•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Assignee | ||
Updated•9 years ago
|
Flags: qe-verify+
Assignee | ||
Comment 22•9 years ago
|
||
Comment on attachment 8639790 [details] [diff] [review]
Patch v2
Approval Request Comment
[Feature/regressing bug #]: Bug 1173728 / Windows 10
[User impact if declined]: Ugly top tab border with lw-themes enabled
[Describe test coverage new/current, TreeHerder]: on m-c and tested locally.
[Risks and why]: Low, straightforward CSS fix.
[String/UUID change made/needed]: none
Attachment #8639790 -
Flags: approval-mozilla-beta?
Comment 23•9 years ago
|
||
Confirming the fix using latest 42.0a1 Nightly, build ID: 20150809030213 on Windows 10 64-bit.
Comment on attachment 8639790 [details] [diff] [review]
Patch v2
Approved. The patch was verified on Nightly, we do want the end-user experience on win10 to be super awesome. Let's uplift to m-b.
Attachment #8639790 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 25•9 years ago
|
||
Comment 26•9 years ago
|
||
Also verified fixed on Firefox 41.0b2, build ID: 20150817163452.
You need to log in
before you can comment on or make changes to this bug.
Description
•