tracking-protection-16.svg is loaded in windows that don't need it

RESOLVED FIXED in Firefox 59

Status

()

P4
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: dao, Assigned: prathiksha)

Tracking

(Blocks: 2 bugs, {perf})

Trunk
Firefox 59
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: [reserve-photon-performance])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
<jwatt> I'm seeing chrome://browser/skin/tracking-protection-16.svg#enabled load during startup
<jwatt> loading the start page
<jwatt> but I'm not seeing that displayed
<jwatt> do you know where I'd see it?
<dao> in the identity block in private windows
<dao> or with privacy.trackingprotection.enabled = true in non-private windows
<jwatt> huh
<jwatt> I wonder why that's loading during startup when no private windows are opened
<dao> I think it's because we slide in the icon rather than hiding it. :/ http://searchfox.org/mozilla-central/source/browser/themes/shared/identity-block/identity-block.inc.css#140-150
(Reporter)

Updated

2 years ago
Keywords: perf
Potentially something for photon-perf though we don't know how much impact this really has.
Blocks: 1348289
Whiteboard: [photon-performance]
Flags: qe-verify?
Priority: -- → P2
Priority: P2 → P3
Whiteboard: [photon-performance] → [reserve-photon-performance]
Flags: qe-verify? → qe-verify-
No longer blocks: 1348289
Priority: P3 → P4
(Assignee)

Updated

a year ago
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 4

a year ago
mozreview-review
Comment on attachment 8939889 [details]
Bug 1363040 - Stop loading tracking-protection-16.svg in windows that don't display it.

https://reviewboard.mozilla.org/r/210198/#review215910

Thanks for tackling this!

::: browser/themes/shared/identity-block/identity-block.inc.css:177
(Diff revision 1)
>    margin-inline-end: -20px;
>    pointer-events: none;
>    opacity: 0;
>    /* Only animate the shield in, when it disappears hide it immediately. */
>    transition: none;
> +  display: none;

While this should solve the problem, we now have the issue that the tracking protection icon is skipping its animation, which it should not.

Another approach could be unsetting the list-style-image instead of using display: none. That could have the same effect but retain the animation.

Can you try that? :)

Thanks!
Attachment #8939889 - Flags: review?(jhofmann)
Comment hidden (mozreview-request)

Comment 6

a year ago
mozreview-review
Comment on attachment 8939889 [details]
Bug 1363040 - Stop loading tracking-protection-16.svg in windows that don't display it.

https://reviewboard.mozilla.org/r/210198/#review216278

Amazing, thank you!

::: browser/themes/shared/identity-block/identity-block.inc.css:177
(Diff revision 2)
>    margin-inline-end: -20px;
>    pointer-events: none;
>    opacity: 0;
>    /* Only animate the shield in, when it disappears hide it immediately. */
>    transition: none;
> +  list-style-image: unset;

Nit: list-style-image: none might be a bit more accurate since we don't want to reset the image to the parent value but just avoid displaying anything at all.
Attachment #8939889 - Flags: review?(jhofmann) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Keywords: checkin-needed
The issue still open in Review Board has to set to fixed else Review Board will prevent the landing of the patch.
Flags: needinfo?(prathikshaprasadsuman)
Keywords: checkin-needed
(Assignee)

Updated

a year ago
Flags: needinfo?(prathikshaprasadsuman)
Keywords: checkin-needed

Comment 9

a year ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/9d19176e7d25
Stop loading tracking-protection-16.svg in windows that don't display it. r=johannh
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9d19176e7d25
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.