Closed Bug 1173730 Opened 5 years ago Closed 4 years ago

Use solid tab separator image on Windows 8 and 10

Categories

(Firefox :: Theme, defect, P1)

Unspecified
Windows 10
defect

Tracking

()

VERIFIED FIXED
Firefox 41
Tracking Status
firefox39 --- wontfix
firefox40 - verified
firefox41 --- verified
firefox42 --- verified

People

(Reporter: dao, Assigned: ntim)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 4 obsolete files)

Need to figure out if we need to actually use an image for this or whether plain CSS can do the job.
I think Stephen wanted this as well on Windows 8. See https://people.mozilla.org/~shorlander/mockups-interactive/australis-interactive-mockups/windows8.html

Also we can use a solid linear gradient here :

#tabbrowser-tabs[movingtab] > .tabbrowser-tab[beforeselected]:not([last-visible-tab])::after, .tabbrowser-tab:not([selected]):not([afterselected-visible]):not([afterhovered]):not([first-visible-tab]):not(:hover)::before, #tabbrowser-tabs:not([overflow]) > .tabbrowser-tab[last-visible-tab]:not([selected]):not([beforehovered]):not(:hover)::after {
	background-image: linear-gradient(transparent 0%, transparent 15%, rgba(100,100,100,0.3) 15%, rgba(100,100,100,0.3) 90%, transparent 90%) !important;
	background-size: 1px 100% !important;

}
Assignee: nobody → ntim.bugs
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
Untested.
Comment on attachment 8626647 [details] [diff] [review]
Patch

This works fine on Windows 10. Haven't tested on older Win versions yet.
Attachment #8626647 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8626647 [details] [diff] [review]
Patch

>+/* Use solid tab separator for Windows 8+ */
>+@media not all and (-moz-os-version: windows-xp) {
>+  @media not all and (-moz-os-version: windows-vista) {
>+    @media not all and (-moz-os-version: windows-win7) {
>+      :root {
>+        --tab-separator-image: linear-gradient(transparent 0%, transparent 15%, rgba(100,100,100,0.3) 15%, rgba(100,100,100,0.3) 90%, transparent 90%);
>+        --tab-separator-size: 1px 100%;
>+      }
>+    }
>+  }
>+}

Not sure whether that works better or worse than the current image for High Contrast and dark themes. I would guess it's worse. In any case, let's use this opportunity to improve the situation there. You can just use threedshadow instead of the rgba value for #TabsToolbar[brighttext], and I think we should do that regardless of the OS and OS version.
Attachment #8626647 - Flags: review?(gijskruitbosch+bugs) → review-
(In reply to Dão Gottwald [:dao] from comment #5)
> You can just use
> threedshadow instead of the rgba value for #TabsToolbar[brighttext],

Oh wait, theedshadow would work for High Contrast but not for dark lightweight themes. So we'll actually need to use some rgb or rgba value close to white.
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #8626647 - Attachment is obsolete: true
Attachment #8626707 - Flags: review?(dao)
Comment on attachment 8626707 [details] [diff] [review]
Patch v2

>+/* Use solid tab separator for Windows 8+ */
>+@media not all and (-moz-os-version: windows-xp) {
>+  @media not all and (-moz-os-version: windows-vista) {
>+    @media not all and (-moz-os-version: windows-win7) {
>+      :root {
>+        --tab-separator-image: linear-gradient(transparent 0%, transparent 15%, currentColor 15%, currentColor 90%, transparent 90%);
>+        --tab-separator-size: 1px 100%;
>+        --tab-separator-opacity: 0.25;
>+      }
>+    }
>+  }
>+}

That's a neat idea, but I suspect a 25% opacity is lower than what we'd want for dark themes and high contrast themes in particular.

Also, like I said I think we'd want to use the white separator for dark themes on all OSes. So, can you set the variable on #TabsToolbar rather than :root and do as I suggested in my previous comments?
Attachment #8626707 - Flags: review?(dao)
Attached patch Patch v3 (obsolete) — Splinter Review
Attachment #8626707 - Attachment is obsolete: true
Attachment #8626893 - Flags: review?(dao)
Comment on attachment 8626893 [details] [diff] [review]
Patch v3

> /* Background tab separators (3px wide).
>    Also show separators beside the selected tab when dragging it. */
> #tabbrowser-tabs[movingtab] > .tabbrowser-tab[beforeselected]:not([last-visible-tab])::after,
> .tabbrowser-tab:not([visuallyselected]):not([afterselected-visible]):not([afterhovered]):not([first-visible-tab]):not(:hover)::before,
> #tabbrowser-tabs:not([overflow]) > .tabbrowser-tab[last-visible-tab]:not([visuallyselected]):not([beforehovered]):not(:hover)::after {
>   -moz-margin-start: -1.5px;
>   -moz-margin-end: -1.5px;
>-  background-image: url(chrome://browser/skin/tabbrowser/tab-separator.png);
>+  background-image: var(--tab-separator-image);
>   background-position: left bottom var(--tab-toolbar-navbar-overlap);
>   background-repeat: no-repeat;
>-  background-size: 3px 100%;
>+  background-size: var(--tab-separator-size);
>+  opacity: var(--tab-separator-opacity);
>   content: "";
>   display: -moz-box;
>   width: 3px;
> }

I assume leaving the horizontal margin and width as is fine. Although technically they could end up too small if --tab-separator-size was set to >3px...

>+/* Use bright solid tab separators for lw-themes */
>+#TabsToolbar[brighttext] {
>+  --tab-separator-image: linear-gradient(transparent 0%, transparent 15%, currentColor 15%, currentColor 90%, transparent 90%);
>+  --tab-separator-size: 1px 100%;
>+  --tab-separator-opacity: 0.4;
>+}

nit: The comment isn't quite correct. You can just remove it.

>+/* Use solid tab separators for Windows 8+ */
>+@media not all and (-moz-os-version: windows-xp) {
>+  @media not all and (-moz-os-version: windows-vista) {
>+    @media not all and (-moz-os-version: windows-win7) {
>+      #TabsToolbar {
>+        --tab-separator-image: linear-gradient(transparent 0%, transparent 15%, currentColor 15%, currentColor 90%, transparent 90%);
>+        --tab-separator-size: 1px 100%;
>+        --tab-separator-opacity: 0.2;
>+      }
>+    }
>+  }
>+}

Please add :not([brighttext]) here.

> /* Use lighter colors of buttons and text in the titlebar on luna-blue */
> @media (-moz-windows-theme: luna-blue) {
>-  #tabbrowser-tabs[movingtab] > .tabbrowser-tab[beforeselected]:not([last-visible-tab])::after,
>-  .tabbrowser-tab:not([visuallyselected]):not([afterselected-visible]):not([afterhovered]):not([first-visible-tab]):not(:hover)::before,
>-  #tabbrowser-tabs:not([overflow]) > .tabbrowser-tab[last-visible-tab]:not([visuallyselected]):not([beforehovered]):not(:hover)::after {
>-    background-image: url("chrome://browser/skin/tabbrowser/tab-separator-luna-blue.png");
>+  #TabsToolbar {
>+    --tab-separator-image: url("chrome://browser/skin/tabbrowser/tab-separator-luna-blue.png");
>   }
> }

And here too.

Thanks!
Attachment #8626893 - Flags: review?(dao) → review+
Attached patch Patch v4 (r=dao) (obsolete) — Splinter Review
Addressed comments.
Attachment #8626893 - Attachment is obsolete: true
Attachment #8626899 - Flags: review+
Keywords: checkin-needed
Keywords: checkin-needed
Attachment #8626899 - Attachment is obsolete: true
Attachment #8626910 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b0ac036f4c7f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment on attachment 8626910 [details] [diff] [review]
Patch v4.1 (r=dao)

Approval Request Comment (for Firefox 40)
[Feature/regressing bug #]: Windows 10 styling work (Firefox 40 will be the first release after Windows 10's launch)
[User impact if declined]: tab styling will feel less integrated in win 10
[Describe test coverage new/current, TreeHerder]: on m-c
[Risks and why]: Low, css only change
[String/UUID change made/needed]: none.
Attachment #8626910 - Flags: approval-mozilla-beta?
[Tracking Requested - why for this release]: See previous comment
Depends on: 1179756
Verified fixed on Nightly 42.0a1 (2015-06-30) and Aurora 41.0a2 (2015-07-06), using Windows 10 Pro x64 (Insider Preview Build 10158).

The tab separators displayed in the tab strip are now displayed in a solid gray.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Comment on attachment 8626910 [details] [diff] [review]
Patch v4.1 (r=dao)

Verified visual fix for Windows 10. Beta+
Attachment #8626910 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Not going to track this bug (not tracking every style change) but it is approved for beta.
Flags: qe-verify+
Verified fixed on Beta 40.0b2 (20150706172413) as well, using Windows 10 Pro x64 (Build 10158).
Flags: qe-verify+
Depends on: 1189212
Summary: Use solid tab separator image on Windows 10 → Use solid tab separator image on Windows 8 and 10
Depends on: 1181907
You need to log in before you can comment on or make changes to this bug.