1477350 - Blocked autoplay icon does not use light colors in dark themes

Status: RESOLVED FIXED

(Firefox :: Site Identity, defect, P2)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot of bug

See attached screenshot. The icon should be using the white/light-gray color that the other identity icons are using.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Screenshot #2 of the bug

Also in the popup.

Comment 2

[Johann Hofmann [:johannh]]

Huh, I thought I had checked this stuff in my review, must have missed that one.

In any case, Dale, would you like to take this?

Comment 3

[Dale Harvey (:daleharvey)]

Yuo also thought I had checked context-fill values, will take

Comment 4

[Dale Harvey (:daleharvey)]

Bryant, if I take out the fill= and stroke= from https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/notification-icons/autoplay-media-blocked.svg and give it context-stroke= it still ends up looking wrong this is the only icon using stroke in the url bar so I think it should probably be reworked to use fill only

https://i.imgur.com/QM3jE7D.png

Comment 5

[Mark Liang(:mark_liang)]

Hi Dale, can you try if this one works? -> https://send.firefox.com/download/d38e95539a/#EBhzpH3RR1WHgirbUf1y1A
Thanks!

Comment 6

[Dale Harvey (:daleharvey)]

Yup thats great, now I can see that the original was also broken on the default theme as well, will fix, cheers

Comment 7

[Dale Harvey (:daleharvey)]

Attachment: Bug 1477350 - Fix autoplay blocked icon to use context-fill correctly.

Review commit: https://reviewboard.mozilla.org/r/258988/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/258988/

Comment 8

[Tim Nguyen :ntim]

Comment on attachment 8994434 [details]
Bug 1477350 - Fix autoplay blocked icon to use context-fill correctly.

https://reviewboard.mozilla.org/r/258988/#review265990

::: browser/themes/shared/notification-icons/autoplay-media-blocked.svg:4
(Diff revision 1)
>  <!-- This Source Code Form is subject to the terms of the Mozilla Public
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16" fill="context-fill" fill-opacity="context-fill-opacity">

I've optimized it further: https://gist.github.com/nt1m/29f0e5b592db114a6fd4f394910d6015

Comment 9

[Dale Harvey (:daleharvey)]

Comment on attachment 8994434 [details]
Bug 1477350 - Fix autoplay blocked icon to use context-fill correctly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/258988/diff/1-2/

Comment 10

[Dale Harvey (:daleharvey)]

Nice thanks, updated the PR with yours

Comment 11

[Florian Quèze [:florian]]

Comment on attachment 8994434 [details]
Bug 1477350 - Fix autoplay blocked icon to use context-fill correctly.

https://reviewboard.mozilla.org/r/258988/#review266134

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/caf5ba52d57e
Fix autoplay blocked icon to use context-fill correctly. r=florian

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 14

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 63.0a1 (2018-07-20) on Windows 10, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID 	20180825100331
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0