Replace reload/stop assets to account for transparency

VERIFIED FIXED in Firefox 57

Status

()

Firefox
General
P1
normal
VERIFIED FIXED
6 months ago
5 months ago

People

(Reporter: epang, Assigned: jaws)

Tracking

57 Branch
Firefox 57
Points:
---

Firefox Tracking Flags

(firefox57 verified)

Details

(Whiteboard: [reserve-photon-animation])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

6 months ago
Created attachment 8899981 [details]
SVGs + JSONs.zip

Replace the svg sprite asset so it works with transparency.

Updated

6 months ago
Flags: qe-verify-

Updated

6 months ago
Flags: qe-verify- → qe-verify+
Priority: -- → P4
QA Contact: stefan.georgiev
Summary: [ux] Replace refresh/stop assets to account for transparency → Replace refresh/stop assets to account for transparency
Whiteboard: [photon-animation] [triage] [ux] → [reserve-photon-animation]
(Reporter)

Updated

6 months ago
Priority: P4 → P3
Assignee: nobody → jaws
Status: NEW → ASSIGNED

Updated

6 months ago
Iteration: --- → 57.3 - Sep 19
Priority: P3 → P1
Summary: Replace refresh/stop assets to account for transparency → Replace reload/stop assets to account for transparency
Comment hidden (mozreview-request)

Comment 2

6 months ago
mozreview-review
Comment on attachment 8902936 [details]
Bug 1392793 - Replace reload/stop assets to account for transparency.

https://reviewboard.mozilla.org/r/174670/#review179850

Thanks! The SVG are better now, but I did find stuff to remove. :) The cleaned SVG should take in account the opacity, but please let me know if they don't.

r+ with the cleaned SVG used.

::: browser/themes/shared/compacttheme.inc.css:54
(Diff revision 1)
>    --chrome-selection-color: #f5f7fa;
>    --chrome-selection-background-color: #4c9ed9;
>  }
>  
>  .toolbarbutton-animatable-box[brighttext],
> +toolbar[brighttext] .toolbarbutton-animatable-box,

Good catch! I don't know why I haven't noticed this issue until now (even though I use the dark theme daily).

::: browser/themes/shared/icons/reload-to-stop.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="468" height="20" fill="context-fill">
> -  <svg x="0">
> +  <svg>

I've optimized this SVG: https://gist.github.com/nt1m/663b147ec798964fb4089d3bf8faf0c9

Many masks with no visible effects have been removed, some groups have been flattened.

::: browser/themes/shared/icons/stop-to-reload.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="468" height="20" fill="context-fill">

I've optimized this SVG as well: https://gist.github.com/nt1m/663b147ec798964fb4089d3bf8faf0c9#file-stop-to-reload-svg
Attachment #8902936 - Flags: review?(ntim.bugs) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

6 months ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8676b5208a5
Replace reload/stop assets to account for transparency. r=ntim

Comment 7

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d8676b5208a5
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Updated

6 months ago
Duplicate of this bug: 1394680

Comment 9

5 months ago
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 (20170915100121)

This issue is verified as fixed with the latest Nightly build on 9/15/2017.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified

Updated

5 months ago
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.