Use a chrome icon for chrome URLs in the location bar

RESOLVED FIXED in Firefox 25

Status

()

RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: marco, Assigned: jaws)

Tracking

Trunk
Firefox 25
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

7 years ago
For in-content pages (like about:home) it would be better to remove the generic icon, as they are obviously secure.
Bug 685059 may fix this.
I don't think we can simply remove the generic (globe) icon for in-content pages. Doing so would remove the ability to drag the icon to create bookmarks or open in a new tab. However, we could use a different icon for chrome URLs.
Summary: Remove the generic icon for in-content pages → Use a chrome icon for chrome URLs in the location bar
(In reply to Jared Wein [:jaws] from comment #2)
> I don't think we can simply remove the generic (globe) icon for in-content
> pages. Doing so would remove the ability to drag the icon to create
> bookmarks or open in a new tab. However, we could use a different icon for
> chrome URLs.

I agree, but this disagrees with https://bugzilla.mozilla.org/show_bug.cgi?id=685059#c24 (that on the other side was posted before all of the identity box redesign), may you figure out with Stephen what's the final plan?
That bug is for the tab bar. We talked about removing the site identity block for chrome URLs at the work week but decided against it for the reasons mentioned in comment #2.
(In reply to Jared Wein [:jaws] from comment #4)
> That bug is for the tab bar.

whoops, my fault. thanks.
Maybe we could use a Home icon for this?

Comment 7

7 years ago
Or a gear icon, this matches most "about:*"s. Esp. interesting when in-content prefs land. 

However, some pages could use even another icon, e. g. the suggested home icon (only) for about:newtab and about:home.
Could also use a yellow /!\ icon for network error pages, and bring back the red/white "do not enter" icon for the malware/phishing blocking page.

Updated

6 years ago
Blocks: 752447

Updated

6 years ago
No longer blocks: 752447
(Reporter)

Updated

6 years ago
Keywords: uiwanted
Depends on: 869500

Comment 9

5 years ago
You can see what it should look like on this mockup : icon + Firefox (in orange) | about:*

http://people.mozilla.com/~shorlander/customizationMode-liveDemo-i02/windows7-customizationMode-i02.html

Comment 10

5 years ago
Great work! :)

How will this affect special pages like about:neterror and the phishing warning (I see these pages have been removed, at least from about:about)? They should (IMO) get special icons (not Favicons!), e. g. the yellow warning triangle resp. stop sign – showing the same as mocked up in Comment 9 may confuse users.
Stephen, I see http://people.mozilla.com/~shorlander/customizationMode-liveDemo-i02/images-win7/identityBlock-firefox.png. Do you have a HiDPI version?

This shouldn't be too hard to implement once we get that other icon and maybe hover/active states if necessary.

FWIW, I don't think hover/active states here are necessary, since I don't know what we would show if the user clicked on it, unless we wanted the states for drag/drop purposes.
Flags: needinfo?(shorlander)

Comment 12

5 years ago
A "secure" panel may be useful (like Chrome).

Comment 13

5 years ago
(In reply to Florian Bender from comment #10)
> Great work! :)
> 
> How will this affect special pages like about:neterror and the phishing
> warning (I see these pages have been removed, at least from about:about)?
> They should (IMO) get special icons (not Favicons!), e. g. the yellow
> warning triangle resp. stop sign – showing the same as mocked up in Comment
> 9 may confuse users.

Of course this will only be in use for in-content pages like customize, add-ons or preferences and library in the future. Phishing and neterror will keep their special favicon.

Comment 14

5 years ago
(In reply to Guillaume C. [:ge3k0s] from comment #13)
> Phishing and neterror will keep their special favicon.

Favicons can be spoofed (for this reason separate HTTPS icons were introduced) and may be used to trick users into clicking buttons to initiate an attack (or input credentials) while they believe they are on a Firefox error page. Thus, those pages should get their own icons (+ text) though not the "Firefox" one. 

Shall I file a follow up bug or do you consider implementing those in this Bug, too?
(In reply to Florian Bender from comment #14)
> (In reply to Guillaume C. [:ge3k0s] from comment #13)
> > Phishing and neterror will keep their special favicon.
> 
> Favicons can be spoofed (for this reason separate HTTPS icons were
> introduced) and may be used to trick users into clicking buttons to initiate
> an attack (or input credentials) while they believe they are on a Firefox
> error page. Thus, those pages should get their own icons (+ text) though not
> the "Firefox" one. 

The icon in the address bar cannot be changed by a webpage.

Comment 16

5 years ago
But Favicons can, which Guillaume was talking about. Plus, in the current Beta at least, those icons are only used as Favicons, not "identity" icons in the Adress Bar.
(In reply to Jared Wein [:jaws] from comment #11)
> Stephen, I see
> http://people.mozilla.com/~shorlander/customizationMode-liveDemo-i02/images-
> win7/identityBlock-firefox.png. Do you have a HiDPI version?
> 
> This shouldn't be too hard to implement once we get that other icon and
> maybe hover/active states if necessary.
> 
> FWIW, I don't think hover/active states here are necessary, since I don't
> know what we would show if the user clicked on it, unless we wanted the
> states for drag/drop purposes.

We could show a larry like panel: http://people.mozilla.com/~shorlander/bugs/bug750106-firefoxPanel.png
Flags: needinfo?(shorlander)
Created attachment 762304 [details]
Chrome URL Identity Icons - 01
Blocks: 872617
Whiteboard: [Australis:M?]
Created attachment 762349 [details] [diff] [review]
Patch (part 1)

This patch makes use of the new icon and gets it looking all spiffy, but doesn't include the identity panel doorhanger.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #762349 - Flags: review?(mconley)
Also, no reason we can't land this on m-i instead of ux. Stephen, should this wait for Australis or go out in 24?
Flags: needinfo?(shorlander)
Created attachment 762394 [details] [diff] [review]
Patch (part 2)

This patch implements the identity panel bits. I copied mozicon128.png to the content so that we would have an equivalent HiDPI graphic for each of the branding outputs.
Attachment #762394 - Flags: review?(mconley)
(In reply to Jared Wein [:jaws] from comment #21)
> Also, no reason we can't land this on m-i instead of ux. Stephen, should
> this wait for Australis or go out in 24?

Shipping it sooner would be awesome!
Flags: needinfo?(shorlander)
Comment on attachment 762349 [details] [diff] [review]
Patch (part 1)

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

::: browser/themes/linux/browser.css
@@ +841,5 @@
>    -moz-image-region: rect(0, 16px, 16px, 0);
>  }
>  
> +.chromeUI > #page-proxy-favicon[pageproxystate="valid"] {
> +  list-style-image: url(chrome://browser/skin/identity-icons-firefox.png);

It seems to me like this icon should be in the branding directory as it's pretty close to the trademarked logo. Then we would need another version for unofficial branding or use the generic version. I wouldn't hurt to get superreview on branding-related changes so I'm flagging Gavin.
Attachment #762349 - Flags: superreview?(gavin.sharp)
Attachment #762349 - Flags: feedback-
Comment on attachment 762394 [details] [diff] [review]
Patch (part 2)

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

> diff --git a/browser/branding/aurora/mozicon128.png b/browser/branding/aurora/content/icon128.png
> diff --git a/browser/branding/nightly/mozicon128.png b/browser/branding/nightly/content/icon128.png
> diff --git a/browser/branding/official/mozicon128.png b/browser/branding/official/content/icon128.png
> diff --git a/browser/branding/unofficial/mozicon128.png b/browser/branding/unofficial/content/icon128.png

Copying the images in HG feels dirty to me as I dislike forks. Moving them to /content/ or copying them at build time seems like less of a maintenance issue.
Comment on attachment 762349 [details] [diff] [review]
Patch (part 1)

>+#urlbar[pageproxystate="valid"] > .chromeUI,
> #urlbar[pageproxystate="valid"] > #identity-box.verifiedIdentity {

#identity-box.chromeUI

>+#urlbar[pageproxystate="valid"] > .chromeUI {

ditto
Comment on attachment 762394 [details] [diff] [review]
Patch (part 2)

> /* Identity UI */
>+#identity-popup-content-box:not(.chromeUI) > #identity-popup-brandName,
>+#identity-popup-content-box:not(.chromeUI) > #identity-popup-chromeLabel,
> #identity-popup-content-box.unknownIdentity > #identity-popup-connectedToLabel ,
> #identity-popup-content-box.unknownIdentity > #identity-popup-runByLabel ,
> #identity-popup-content-box.unknownIdentity > #identity-popup-content-host ,
> #identity-popup-content-box.unknownIdentity > #identity-popup-content-owner ,
> #identity-popup-content-box.verifiedIdentity > #identity-popup-connectedToLabel2 ,
> #identity-popup-content-box.verifiedDomain > #identity-popup-connectedToLabel2 {
>   display: none;
> }
> 
>+#identity-popup-content-box.chromeUI > .identity-popup-label:not(#identity-popup-brandName):not(#identity-popup-chromeLabel),
>+#identity-popup-content-box.chromeUI > .identity-popup-description,
>+#identity-popup-content-box.chromeUI > #identity-popup-button-container {
>+  display: none;
>+}

Why is this stuff split across two rules?
(In reply to Matthew N. [:MattN] from comment #24)
> It seems to me like this icon should be in the branding directory as it's
> pretty close to the trademarked logo. Then we would need another version for
> unofficial branding or use the generic version.

That's a good idea. Stephen, can create a HiDPI version of the graphics with all three states for consistency with Linux/Windows?

Also would be cool if we had nightly/aurora/(and shriek, unofficial!) versions too. But for unofficial I don't see a problem with continuing to use the unknownIdentity globe that we currently use.
Flags: needinfo?(shorlander)
Comment on attachment 762349 [details] [diff] [review]
Patch (part 1)

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

This looks sane, but withholding an r+ until we hear more about branding rules.

I also think there are sections that could be put into browser/themes/shared, since they seem common.

::: browser/themes/osx/browser.css
@@ +1251,5 @@
>    padding-right: 10.01px;
>  }
>  
> +#urlbar[pageproxystate="valid"] > .chromeUI,
> +#urlbar[pageproxystate="valid"] > #identity-box.verifiedIdentity {

Seems like this rule could go into ../shared. Same with the one in windows/browser.css. linux/browser.css has something similar too - but with an added background-color that we can just override with.
Attachment #762349 - Flags: review?(mconley)
Comment on attachment 762394 [details] [diff] [review]
Patch (part 2)

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

This looks good to me - just a few suggestions (mostly with putting things into browser/themes/shared).

::: browser/base/content/browser.css
@@ +394,5 @@
>  }
>  
>  /* Identity UI */
> +#identity-popup-content-box:not(.chromeUI) > #identity-popup-brandName,
> +#identity-popup-content-box:not(.chromeUI) > #identity-popup-chromeLabel,

As Dao suggested, we might as well merge these two rule blocks.

::: browser/base/content/browser.js
@@ +6544,5 @@
> +      let brandBundle = document.getElementById("bundle_brand");
> +      let brandShortName = brandBundle.getString("brandShortName");
> +      this._identityPopupChromeLabel.textContent = gNavigatorBundle.getFormattedString("identity.chrome",
> +                                                                                       [brandShortName]);
> +    }

Has something gone wrong with the indentation here? It's strange to see two close curly parentheses directly on top of one another...

::: browser/themes/linux/browser.css
@@ +945,5 @@
>    -moz-image-region: rect(128px, 64px, 192px, 0px);
>  }
>  
> +#identity-popup.chromeUI > #identity-popup-container > #identity-popup-icon {
> +  list-style-image: url("chrome://branding/content/icon64.png");

This can probably go into a shared include as well.

::: browser/themes/windows/browser.css
@@ +1830,5 @@
>    -moz-padding-start: 15px;
>    margin: 0;
>  }
>  
> +#identity-popup-brandName {

Here's another thing that could be put into a common file under browser/themes/shared.
Attachment #762394 - Flags: review?(mconley)
Created attachment 765105 [details]
Chrome URL Identity Icon @2x

(In reply to Jared Wein [:jaws] from comment #28)
> (In reply to Matthew N. [:MattN] from comment #24)
> > It seems to me like this icon should be in the branding directory as it's
> > pretty close to the trademarked logo. Then we would need another version for
> > unofficial branding or use the generic version.
> 
> That's a good idea. Stephen, can create a HiDPI version of the graphics with
> all three states for consistency with Linux/Windows?

Attached.

> Also would be cool if we had nightly/aurora/(and shriek, unofficial!)
> versions too. But for unofficial I don't see a problem with continuing to
> use the unknownIdentity globe that we currently use.

I would prefer to do that in a follow-up if possible.
Flags: needinfo?(shorlander)
Created attachment 765195 [details] [diff] [review]
Patch v2

This patch updates the icons to be used, moved them to branding, split out the CSS to a shared file, and includes a kitten.

I agree with Stephen, that we can update the icons for unofficial, nightly, and aurora in a follow-up.
Attachment #762349 - Attachment is obsolete: true
Attachment #762394 - Attachment is obsolete: true
Attachment #762349 - Flags: superreview?(gavin.sharp)
Attachment #765195 - Flags: superreview?(gavin.sharp)
Attachment #765195 - Flags: review?(mnoorenberghe+bmo)
Created attachment 765198 [details] [diff] [review]
Patch v2.1

Forgot to hg add the identity-icons-brand.png files.
Attachment #765195 - Attachment is obsolete: true
Attachment #765195 - Flags: superreview?(gavin.sharp)
Attachment #765195 - Flags: review?(mnoorenberghe+bmo)
Attachment #765198 - Flags: superreview?(gavin.sharp)
Attachment #765198 - Flags: review?(mnoorenberghe+bmo)
Whiteboard: [Australis:M?] → [Australis:M?][Australis:P5]
Attachment #765198 - Flags: review?(mnoorenberghe+bmo) → review?(mconley)
Comment on attachment 765198 [details] [diff] [review]
Patch v2.1

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

Yeah, this looks fine. Thanks Jared!
Attachment #765198 - Flags: review?(mconley) → review+
Comment on attachment 765198 [details] [diff] [review]
Patch v2.1

I filed bug 888563 to track updating the images for unofficial, Nightly, and Aurora. I changed the unofficial branding to use the globe instead of the Firefox icon so it can remain generic. This should drop the need for s-r since it won't introduce any branding issues.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3c5133764596
Attachment #765198 - Flags: superreview?(gavin.sharp)

Comment 36

5 years ago
Shouldn't this be also displayed on new tab or will it be too distracting for the user ?
https://hg.mozilla.org/mozilla-central/rev/3c5133764596
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
(In reply to Jared Wein [:jaws] from comment #35)
> I changed the unofficial branding to use the globe instead of the
> Firefox icon so it can remain generic. This should drop the need for s-r
> since it won't introduce any branding issues.

FWIW, the unofficial branding package is currently unused, so this did introduce branding issues for Nightly/Aurora. We'll probably fix that as part of bug 840883, though.

Updated

5 years ago
Depends on: 889493
Whiteboard: [Australis:M?][Australis:P5]

Updated

5 years ago
Blocks: 889428
Depends on: 891833

Updated

5 years ago
No longer depends on: 891833
This is already on m-c, so not tracking for Australis.
No longer blocks: 872617

Updated

5 years ago
Depends on: 908534

Updated

5 years ago
Blocks: 922356

Updated

4 years ago
Depends on: 994754
You need to log in before you can comment on or make changes to this bug.