1375893 - Update pinned-tab highlight/glow style for Photon

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Justin Dolske [:Dolske]]

The current highlight/glow on pinned tabs (used as a notification to indicate when the title has changed) doesn't work so well with the dark titlebar from bug 1367384 / bug 1367385. The Photon mocks show this as changing to a small blue dot.

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

I checked with Stephen -- this is the SVG from his demo mockup, and is fine to use. And the single image/color is fine for the default/light/dark themes.

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8882094 [details] [diff] [review]
Patch v.1

>--- a/browser/themes/shared/compacttheme.inc.css
>+++ b/browser/themes/shared/compacttheme.inc.css

>+%ifdef MOZ_PHOTON_THEME
>+  --pinned-tab-glow-url: url(chrome://browser/skin/tabbrowser/indicator-tab-attention.svg);

>@@ -84,7 +88,11 @@ toolbar:-moz-lwtheme-brighttext  {
>   --tab-selection-color: #f5f7fa;
>   --tab-selection-background-color: #4c9ed9;
>   --tab-selection-box-shadow: none;
>+%ifdef MOZ_PHOTON_THEME
>+  --pinned-tab-glow-url: url(chrome://browser/skin/tabbrowser/indicator-tab-attention.svg);

> .tabbrowser-tab:-moz-any([image], [pinned]) > .tab-stack > .tab-content[attention]:not([selected="true"]),
> .tabbrowser-tab > .tab-stack > .tab-content[pinned][titlechanged]:not([selected="true"]) {
>+%ifdef MOZ_PHOTON_THEME
>+  background-image: var(--pinned-tab-glow-url);
>+  background-position: center bottom -4px;

Please remove background-image and the --pinned-tab-glow-url variable since you're not changing the value from what you already set in tabs.inc.css.

Comment 3

[Justin Dolske [:Dolske]]

Attachment: Patch v.2

Updated for landing.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3af874360c71c0ed9d0dd01dd05de3cfa9171aed

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8e1f6c6ebd6
Update pinned-tab highlight/glow style for Photon. r=dao

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b8e1f6c6ebd6

Comment 6

[Johann Hofmann [:johannh]]

Sweet screenshots:

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=a578ce873d805743283df9a3627372939d8c8b2c&newProject=mozilla-central&newRev=d536973fe668c6c6046fc3fda82e24f3379e3713&filter=fourPinned

Comment 7

[Tomer Cohen :tomer]

(In reply to Justin Dolske [:Dolske] from comment #0)
> The Photon mocks show this as changing to a small blue dot.

This change introduced huge notification background for non-pinned tabs. Is it by purpose? 

See screenshot here: https://bug1383312.bmoattachments.org/attachment.cgi?id=8888959

Comment 8

[Vlad Bacia-Mociran [:VladB]]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170907220212

I would like to verify this issue but I don't know exactly what to follow or what to look for. I've opened Facebook, Facebook Messenger, Skype in the browser (tabs unpinned) but there is no notification or glow. Only the number of notification is displayed in front of the tab title. Is this the expected result?

I have used the Light theme with Compact density.

Comment 9

[Vlad Bacia-Mociran [:VladB]]

Attachment: pin notification cut off

For the pinned tabs the blue dot that represents the notification is displayed but not fully displayed. The bottom part is cut off. Is this also intended?

Comment 10

[Vlad Bacia-Mociran [:VladB]]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170912220343

Verified on Windows 8.1 x64, on Windows 10 x64 and x32, Mac OS 10.12 and Ubuntu 14.04 with latest Nightly 57.0a1 and I cannot reproduce the issue.

Comment 11

[Brindusa Tot, Desktop QA]

Based on previous comment mark this bug verified.

Comment 12

[Justin Dolske [:Dolske]]

(In reply to Vlad Bacia-Mociran [:VladB] from comment #9)
> Created attachment 8905925 [details]
> pin notification cut off
> 
> For the pinned tabs the blue dot that represents the notification is
> displayed but not fully displayed. The bottom part is cut off. Is this also
> intended?

Yes.

(In reply to Vlad Bacia-Mociran [:VladB] from comment #8)

> I would like to verify this issue but I don't know exactly what to follow or
> what to look for. I've opened Facebook, Facebook Messenger, Skype in the
> browser (tabs unpinned) but there is no notification or glow. Only the
> number of notification is displayed in front of the tab title. Is this the
> expected result?

The tab's title text needs to change while it's in the background (since this highlight is meant to indicate unseen changes). IIRC when writing this I just used a little test page that did a setTimeout (to allow me to switch to another page) then then updated the title.

I see this every day in normal usage, so agree it's verified.