Closed Bug 1181541 Opened 6 years ago Closed 6 years ago

Branding identity icons for internal pages should be SVG

Categories

(Firefox :: General, defect, P1)

defect
Points:
3

Tracking

()

RESOLVED FIXED
Firefox 42
Iteration:
42.2 - Jul 27
Tracking Status
firefox42 --- fixed

People

(Reporter: ttaubert, Assigned: MattN)

References

Details

(Keywords: addon-compat, Whiteboard: [fxprivacy] [campaign])

Attachments

(3 files, 1 obsolete file)

With bug 1175678 fixed all the URL bar icons will be SVG. We should do the same for the branding icons, there currently are three different ones: Aurora, Nightly/Unofficial, and Official.

Steven, how difficult would it be to get those in 16px and 24px versions?
Flags: qe-verify-
Flags: needinfo?(shorlander)
Flags: firefox-backlog?
Whiteboard: [fxprivacy]
Whiteboard: [fxprivacy] → [fxprivacy] [ux]
Rank: 4
Flags: firefox-backlog? → firefox-backlog+
Priority: -- → P2
Summary: [ux] Branding identity icons for internal pages should be SVG → [UX] Branding identity icons for internal pages should be SVG
Rank: 4
Attached image firefox.svg (obsolete) —
Here's an SVG. Not knowing what you need it for I have a few concerns. SVGs are not always an appropriate format, and you might actually need a highly tailored SVG file to get good results. 

Can someone send me the mock up of what's being proposed. I might be able to create a better option for you.
This is for http://grab.by/IR7W (e.g. from about:home) which we want to scale for various dppx (e.g. on Windows with 1.25x, 1.5x, etc.)

We're looking for replacements for the files listed here:
https://mxr.mozilla.org/mozilla-central/find?string=identity-icons-brand&tree=mozilla-central&hint=
Flags: needinfo?(bbell)
Here are the SVGs you asked for.
Attachment #8634308 - Attachment is obsolete: true
Flags: needinfo?(shorlander)
Flags: needinfo?(bbell)
Thanks!
Assignee: nobody → MattN+bmo
Blocks: 1023511
Status: NEW → ASSIGNED
Iteration: --- → 42.2 - Jul 27
Points: --- → 3
Rank: 4
Priority: P2 → P1
Whiteboard: [fxprivacy] [ux] → [fxprivacy]
Summary: [UX] Branding identity icons for internal pages should be SVG → Branding identity icons for internal pages should be SVG
Whiteboard: [fxprivacy] → [fxprivacy] [campaign]
I couldn't fine anywhere the 2nd or 3rd state in the sprite were used in the tree but Classic Theme Restorer is using it and this bug isn't making the problem worse.

Marking addon-compat and CC'ing bsmedberg and Aris since their extensions reference the PNG files.
Keywords: addon-compat
Bug 1181541 - Convert branding identity icons for internal pages to SVG. r=ttaubert
Attachment #8636957 - Flags: review?(ttaubert)
(In reply to Matthew N. [:MattN] from comment #5)
> I couldn't fine anywhere the 2nd or 3rd state in the sprite were used in the
> tree but Classic Theme Restorer is using it and this bug isn't making the
> problem worse.
> 
> Marking addon-compat and CC'ing bsmedberg and Aris since their extensions
> reference the PNG files.

Would you prefer the icons on their own?
Attached file Identity-singles.zip
All four icons minus the hover and active states.
Tested on Win8.1 and OS X 10.10 btw.
(In reply to bbell from comment #7)
> (In reply to Matthew N. [:MattN] from comment #5)
> > I couldn't fine anywhere the 2nd or 3rd state in the sprite were used in the
> > tree but Classic Theme Restorer is using it and this bug isn't making the
> > problem worse.
> 
> Would you prefer the icons on their own?

We'll see what Tim says. This bug won't make anything worse either way.

(In reply to bbell from comment #8)
> All four icons minus the hover and active states.

Thanks
Thanks for letting me know.

I will update CTRs brand icons references once SVG icons hit Nightly channel. Just did some local tests with provided SVG files and everything works fine.

Is this change planned for Fx42+ in general (F42: Nightly->Aurora->Beta->Release) or for Nightly only?
Comment on attachment 8636957 [details]
MozReview Request: Bug 1181541 - Convert branding identity icons for internal pages to SVG. r=ttaubert

https://reviewboard.mozilla.org/r/13771/#review12373

The SVG files itself are quite heavy already, I think we shouldn't include the hover/active states if we don't use them anymore. All other location bar SVGs have a single state.

Should we push to try to see whether that makes a difference for Talos?

All SVG files should have the following header:

<?xml version="1.0" encoding="utf-8"?>
<!-- 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/. -->

We should add the following to the top of every SVG file:

<?xml version="1.0" encoding="utf-8"?>
<!-- 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/. -->
Attachment #8636957 - Flags: review?(ttaubert)
Comment on attachment 8636957 [details]
MozReview Request: Bug 1181541 - Convert branding identity icons for internal pages to SVG. r=ttaubert

Bug 1181541 - Convert branding identity icons for internal pages to SVG. r=ttaubert
Attachment #8636957 - Flags: review?(ttaubert)
Try push in progress (waiting for try autoland) for talos.

Aris, note that we're removing 2nd and third state now.
(In reply to Aris from comment #11)
> Thanks for letting me know.

No problem. It's easy when there are only a few affected AMO add-ons and I can figure out the author's bugmail address.

> Is this change planned for Fx42+ in general (F42:
> Nightly->Aurora->Beta->Release) or for Nightly only?

It will land in 42 and ride the trains.
Attachment #8636957 - Flags: review?(ttaubert) → review+
Comment on attachment 8636957 [details]
MozReview Request: Bug 1181541 - Convert branding identity icons for internal pages to SVG. r=ttaubert

https://reviewboard.mozilla.org/r/13771/#review12423

r=me with the tiny CSS fix and the <defs> and <g> elements removed from all four SVGs. Thanks!

::: browser/themes/shared/identity-block/identity-block.inc.css:98
(Diff revision 2)
> -  -moz-image-region: rect(0, 16px, 16px, 0);
> +  -moz-image-region: rect(0, 32px, 32px, 0);

We don't need this any longer.

::: browser/branding/aurora/content/identity-icons-brand.svg:14
(Diff revision 2)
> +  <g id="artboard-1">

<g> is a grouping element, we don't need it.

::: browser/branding/aurora/content/identity-icons-brand.svg:6
(Diff revision 2)
> +  <defs>

No need to wrap <style> in <defs>. Just remove the <defs> element.
Sorry, I didn't attempt to clean up the SVG. I've addressed the comments locally. Just waiting on the Talos results now:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=8e5c888d0d89&newProject=try&newRevision=103193c1cd13

So far there's nothing really bad on Linux.
Windows 7 results have been running for 5 hours and I don't want to wait anymore after all of the talos/perfherder issues plus Win7 was fine on the push from comment 18 so I'm pushing this. No relevant red/green shows on compare-talos.
Whiteboard: [fxprivacy] [campaign] [waiting on talos] → [fxprivacy] [campaign]
https://hg.mozilla.org/mozilla-central/rev/d6e98b1b3a63
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in before you can comment on or make changes to this bug.