Closed Bug 1363040 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Site Identity, enhancement, P4)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: dao, Assigned: prathiksha)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [reserve-photon-performance])

Attachments

(1 file)

<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
Keywords: perf
Potentially something for photon-perf though we don't know how much impact this really has.
Whiteboard: [photon-performance]
Flags: qe-verify?
Priority: -- → P2
Priority: P2 → P3
Whiteboard: [photon-performance] → [reserve-photon-performance]
Flags: qe-verify? → qe-verify-
Priority: P3 → P4
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
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 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+
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
Flags: needinfo?(prathikshaprasadsuman)
Keywords: checkin-needed
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
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.