Update pinned-tab highlight/glow style for Photon

VERIFIED FIXED in Firefox 57

Status

()

P1
normal
VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: Dolske, Assigned: Dolske)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 verified disabled, firefox57 verified)

Details

(Whiteboard: [photon-visual][p2])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
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.
Whiteboard: [photon-visual] → [photon-visual] [triage]
Blocks: 1355767
No longer blocks: 1349555
Flags: qe-verify+
Priority: -- → P2
Whiteboard: [photon-visual] [triage] → [photon-visual][p2]
QA Contact: brindusa.tot
(Assignee)

Comment 1

2 years ago
Created attachment 8882094 [details] [diff] [review]
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.
Assignee: nobody → dolske
Attachment #8882094 - Flags: review?(dao+bmo)
Status: NEW → ASSIGNED
Priority: P2 → P1
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.
Attachment #8882094 - Flags: review?(dao+bmo) → review+

Comment 4

2 years ago
Pushed by jdolske@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8e1f6c6ebd6
Update pinned-tab highlight/glow style for Photon. r=dao

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b8e1f6c6ebd6
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Iteration: --- → 56.2 - Jul 10
(Assignee)

Updated

2 years ago
Depends on: 1379052
(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

Updated

2 years ago
See Also: → bug 1383312
status-firefox57: affected → ---
No longer depends on: 1383449
QA Contact: brindusa.tot → ovidiu.boca
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.
Flags: needinfo?(dolske)
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?
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.
Based on previous comment mark this bug verified.
Status: RESOLVED → VERIFIED
status-firefox56: fixed → verified disabled
status-firefox57: --- → verified
Flags: qe-verify+
(Assignee)

Comment 12

a year ago
(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.
Flags: needinfo?(dolske)
You need to log in before you can comment on or make changes to this bug.