1392541 - Update tracking protection icons

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Tim Nguyen :ntim]

http://design.firefox.com/icons/viewer/#tracking

Comment 1

[[:Towkir] Ahmed]

Hey Tim,
Can I take this one ?

Comment 2

[Tim Nguyen :ntim]

Sure.

Comment 3

[Tim Nguyen :ntim]

Here are the icons to update: https://dxr.mozilla.org/mozilla-central/search?q=path%3Atracking+ext%3Asvg+-path%3Aobj&redirect=false

Comment 4

[[:Towkir] Ahmed]

Attachment: tracking-protection-icon.patch

Please have a look at this one.
Let me know if anything should be updated.
Thanks

Comment 5

[Tim Nguyen :ntim]

Comment on attachment 8911384 [details] [diff] [review]
tracking-protection-icon.patch

Review of attachment 8911384 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch!

I suspect the tracking protection icon color will be wrong in about:privatebrowsing (open a private browsing window).
You'll need to set -moz-context-properties: fill; fill: #ccc; on the image inside the private browsing page CSS.

::: browser/themes/shared/controlcenter/tracking-protection.svg
@@ -35,5 @@
> -  <g id="enabled">
> -    <use class="fieldtext" xlink:href="#shape-shield-outer" mask="url(#mask-shield-cutout)"/>
> -  </g>
> -
> -  <g id="disabled">

The current SVG includes 2 variants: disabled and enabled. You need to separate them into 2 different SVGs.

disabled corresponds to the off icon, and enabled corresponds to the on icon.

Comment 6

[Dão Gottwald [:dao]]

Towkir, are you still working on this?

Comment 7

[[:Towkir] Ahmed]

Yes, I am. had some questions for Tim,
Will ask by tomorrow.

Comment 8

[[:Towkir] Ahmed]

Hi Tim,
is browser/themes/shared/identity-block/tracking-protection-16.svg being used anywhere ? I don't see anything like that, if yes, or even if it is no, what should do about it ?

about   browser/themes/shared/controlcenter/tracking-protection.svg
I think you suggested to make it two separate svg files and use them, any idea why should we not update this one too ?
I have done the rest of your suggestion so,
browser/themes/shared/privatebrowsing/ icons and css are updated and working just fine on my local patch
Let me know what should I do about the others mentioned.

Thanks

Comment 9

[Dão Gottwald [:dao]]

(In reply to [:Towkir] Ahmed from comment #8)
> Hi Tim,
> is browser/themes/shared/identity-block/tracking-protection-16.svg being
> used anywhere ?

It's used here:

http://searchfox.org/mozilla-central/rev/bc6dddb88b1f34b54e22efc205846975fb4c04cb/browser/themes/shared/identity-block/identity-block.inc.css#158-165

Comment 10

[Tim Nguyen :ntim]

(In reply to [:Towkir] Ahmed from comment #8)
> Hi Tim,
> is browser/themes/shared/identity-block/tracking-protection-16.svg being
> used anywhere ?

See comment 9.

If you're asking how to make it appear in the UI, you can go to https://www.socialmediaexaminer.com/ in a private window with tracking protection enabled. The icon will appear in the URL bar.

> I don't see anything like that, if yes, or even if it is no,
> what should do about it ?

Replace it with 2 different files tracking-protection.svg and tracking-protection-off.svg then use those in the CSS.

> about   browser/themes/shared/controlcenter/tracking-protection.svg
> I think you suggested to make it two separate svg files and use them, any
> idea why should we not update this one too ?

We should also update this one. But right now, the SVG contains 2 glyphs in them (the on glyph and the off glyph), which we want to separate in 2 files.

Comment 11

[[:Towkir] Ahmed]

Attachment: tracking-protection-icons.patch

Comment 12

[Stephen Horlander [:shorlander]]

Do we want to update the 16 and the 24 to the new Photon style?

Comment 13

[Tim Nguyen :ntim]

For more context, the 16 one is used for the identity block, the 24 one is used for the control center. There's also a 32x32 version in about:privatebrowsing.

I'm assuming we want to update all of them, but the question is whether we still want separate icons for those different situations, or whether the 16x16 version works everywhere.

Comment 14

[Tim Nguyen :ntim]

Comment on attachment 8920829 [details] [diff] [review]
tracking-protection-icons.patch

Clearing until UX input.

Comment 15

[Johann Hofmann [:johannh]]

I'm replacing the 24px (now apparently 32px) icons in bug 1462470. We can keep this open to replace the smaller ones.

Comment 16

[Johann Hofmann [:johannh]]

I'm going to take this over to complete the TP updates we've been doing as part of bug 1461743.

As I mentioned, only the identity block icon needs updating here.

Comment 17

[Johann Hofmann [:johannh]]

Attachment: Bug 1392541 - Update tracking protection icons in the identity block.

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

Comment 18

[Tim Nguyen :ntim]

Comment on attachment 8986722 [details]
Bug 1392541 - Update tracking protection icons in the identity block.

https://reviewboard.mozilla.org/r/252020/#review258428

::: browser/themes/shared/identity-block/identity-block.inc.css:151
(Diff revision 1)
>  }
>  
>  /* TRACKING PROTECTION ICON */
>  
>  #tracking-protection-icon {
> -  list-style-image: url(chrome://browser/skin/tracking-protection-16.svg#enabled);
> +  list-style-image: url(chrome://browser/skin/tracking-protection.svg);

The reference in browser/themes/shared/customizableui/panelUI.inc.css also needs to be updated.

::: browser/themes/shared/identity-block/tracking-protection-disabled.svg:5
(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">
> +  <path d="M10.513 11.382A4.221 4.221 0 0 1 8 12.987a4.267 4.267 0 0 1-1.41-.578l-1.436 1.437a6.221 6.221 0 0 0 2.734 1.148l.112.012.112-.012a6.244 6.244 0 0 0 4.012-2.427 9.26 9.26 0 0 0 1.8-5.286c.043-.518.063-1.421.076-2.281l-2 2a7.572 7.572 0 0 1-1.487 4.382zm4.194-10.089a1 1 0 0 0-1.414 0l-.537.537a1.808 1.808 0 0 0-.285-.077L8 .985l-4.473.768A1.845 1.845 0 0 0 2 3.575c0 1.025 0 2.867.08 3.706a10.2 10.2 0 0 0 1.079 4.146l-1.866 1.866a1 1 0 1 0 1.414 1.414l12-12a1 1 0 0 0 0-1.414zM4 7c-.049-.54 0-1.675 0-3.3l4-.687 3.048.523-6.4 6.4A9.517 9.517 0 0 1 4 7z"></path>

very small nit: 

<path d="..." />

instead of 

<path d="..." ></path>

::: browser/themes/shared/identity-block/tracking-protection.svg:5
(Diff revision 1)
> +  <path d="M8 15.006l-.112-.012a6.244 6.244 0 0 1-4.012-2.427 9.26 9.26 0 0 1-1.8-5.286C2 6.442 2 4.6 2 3.575a1.845 1.845 0 0 1 1.527-1.822L8 .985l4.471.768A1.845 1.845 0 0 1 14 3.576c0 1.023 0 2.866-.08 3.705a9.26 9.26 0 0 1-1.8 5.286 6.244 6.244 0 0 1-4.012 2.427zM4 3.7C4 5.325 3.951 6.46 4 7a7.572 7.572 0 0 0 1.487 4.382A4.223 4.223 0 0 0 8 12.987a4.221 4.221 0 0 0 2.512-1.605A7.572 7.572 0 0 0 12 7c.049-.54 0-1.675 0-3.3l-4-.685z"></path>
> +  <path d="M8 4.537l-2.5.428c.009.942.03 1.655.062 2a5.765 5.765 0 0 0 1.13 3.53 2.685 2.685 0 0 0 1.3.943H8z"></path>

ditto

Comment 19

[Johann Hofmann [:johannh]]

Ah, of course, great catch, thanks!

Comment 20

[Johann Hofmann [:johannh]]

Comment on attachment 8986722 [details]
Bug 1392541 - Update tracking protection icons in the identity block.

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

Comment 21

[Tim Nguyen :ntim]

Comment on attachment 8986722 [details]
Bug 1392541 - Update tracking protection icons in the identity block.

https://reviewboard.mozilla.org/r/252020/#review258436

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ce33c943102
Update tracking protection icons in the identity block. r=ntim

Comment 23

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/5ce33c943102

Comment 24

[ovidiu boca[:Ovidiu]]

Hi Johann,
I want to verify this issue but I need some additional information: this issue should be verified on all OSes, right? The update tracking protection icons should match the ones from the link in the description http://design.firefox.com/icons/viewer/#tracking? 

Thanks Johann in advance for your help.

Comment 25

[Johann Hofmann [:johannh]]

(In reply to ovidiu boca[:Ovidiu] from comment #24)
> Hi Johann,
> I want to verify this issue but I need some additional information: this
> issue should be verified on all OSes, right? The update tracking protection
> icons should match the ones from the link in the description
> http://design.firefox.com/icons/viewer/#tracking? 
> 
> Thanks Johann in advance for your help.

That is correct, thanks!

Comment 26

[Johann Hofmann [:johannh]]

Oh, also note that the only places where the icon should match the one in that link is in the identity block and in the main ("hamburger") menu. The other tracking protection icons are a bit larger and consequently look a bit different.

Comment 27

[ovidiu boca[:Ovidiu]]

Attachment: protection off.png

Johann, please take a look at the attached files with the actual results, the testing was run on MAac OS X 10.12 with FF Nightly 62.0a1(2018-06-22). Please note that the tracking protection off state doesn't match the picture from the documentation:  http://design.firefox.com/icons/viewer/#tracking

Comment 28

[ovidiu boca[:Ovidiu]]

Attachment: protection on.png

Comment 29

[Johann Hofmann [:johannh]]

(In reply to ovidiu boca[:Ovidiu] from comment #27)
> Created attachment 8987042 [details]
> protection off.png
> 
> Johann, please take a look at the attached files with the actual results,
> the testing was run on MAac OS X 10.12 with FF Nightly 62.0a1(2018-06-22).
> Please note that the tracking protection off state doesn't match the picture
> from the documentation:  http://design.firefox.com/icons/viewer/#tracking

Yup, that's what I meant with "other tracking protection icons" in:

> Oh, also note that the only places where the icon should match the one in that link is in the identity block and in the main ("hamburger") menu. The other tracking protection icons are a bit larger and consequently look a bit different.

This one is in the identity popup, so that's expected :)

Comment 30

[ovidiu boca[:Ovidiu]]

Thanks Johann for clearing this out. 
I verified this issue on Windows 10 x64 and Ubuntu 16.04 with FF Nightly 62.0a1(2018-06-25) and the tracking protection icons are updated. I will mark this as a verified fix.