update blank document default favicon

RESOLVED FIXED in Firefox 8

Status

()

Firefox
Theme
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: fryn, Assigned: Margaret)

Tracking

(Depends on: 1 bug)

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

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

7 years ago
The blank 8.5"x11" white sheet of paper with a dog ear isn't square like most favicons are, and it feels dated.

Stephen made a beautiful dotted rounded square icon. Let's use that! :D
There are no documents, only web pages. *waves hand*
(Reporter)

Comment 2

7 years ago
(In reply to comment #1)
> There are no documents, only web pages. *waves hand*

Tell that to the CSS working group. ;) (Where is my vertical centering‽)

(In reply to comment #0)
> Stephen made a beautiful dotted rounded square icon. Let's use that! :D

s/dotted/dashed/

Can't believe I messed that up.

Comment 3

6 years ago
Is this landed in the ux branch?

Comment 4

6 years ago
No, it takes time to land complex patches, such as the patch required for this bug.
(In reply to comment #3)
> Is this landed in the ux branch?

Yes. http://hg.mozilla.org/projects/ux/rev/880821811d75

Comment 6

6 years ago
Oh ya, earlier it was backed out , thanks :)

Comment 7

6 years ago
Created attachment 541680 [details]
Screenshot (24 June 2011)
Created attachment 543196 [details] [diff] [review]
Update Blank Favicon - 01

Changes the default page favicon to a dotted outline
Attachment #543196 - Flags: review?(gavin.sharp)
Created attachment 543198 [details]
Screenshot
(Assignee)

Comment 10

6 years ago
Comment on attachment 543196 [details] [diff] [review]
Update Blank Favicon - 01

I noticed a few issues with this patch. First, when I did an MXR search for moz-icon://stock/gtk-file?size=menu, it looks like there are other places where we would still want to swap out the GTK icon for the default favicon, like in aboutSessionRestore.css and aboutPermissions.css, among others.

Also, I'm not sure we want to change the folder item icons, since it looks like those would be used to represent documents in file systems, not favicons.
(Assignee)

Comment 11

6 years ago
Created attachment 543938 [details] [diff] [review]
alternate patch

This patch keeps the folder item icons the same, but it changes browser.css for pinstripe and winstripe to use defaultFavicon.png instead of the folder item icons for .tab-icon-image and the icons in the all tabs menu. I'm not sure why we currently use the folder item icon in those places, since those icons are supposed to represent favicons. It seems to me like it was just used because it was the same image as the default favicon, so no one necessarily noticed a problem with it.

I also made additional changes to gnomestripe to use the default favicon instead of the GTK stock icon in other places where the default favicon is expected.
Attachment #543938 - Flags: review?(gavin.sharp)
(In reply to comment #11)
> Created attachment 543938 [details] [diff] [review] [review]

If you can explain in a way that people not familiar with the implementation details can parse, I'm happy to UI-review this. I guess I haven't hit this particular edge case? :)
(In reply to comment #12)
> (In reply to comment #11)
> > Created attachment 543938 [details] [diff] [review] [review] [review]
> 
> If you can explain in a way that people not familiar with the implementation
> details can parse, I'm happy to UI-review this. I guess I haven't hit this
> particular edge case? :)

We currently have a sprite that contains the folders and file icons in addition to another file that serves as the default favicon. We are currently using the same blank page image for both.

My patch replaced them all with an outline but Margaret's patch just replaces the default favicon leaving the file images intact. Which is the correct approach.

Although I am not sure that we should use folder-item.png for this anyway. We have some redundant icons in toolkit/dirListing. It would be nice to have a single source for these things.
Attachment #543196 - Flags: review?(gavin.sharp)
Comment on attachment 543938 [details] [diff] [review]
alternate patch

Seems like the simplest thing to do as a first step would be to only change the image used on tabs, and nothing else. Would that inconsistency be too horrible?
Attachment #543938 - Flags: review?(gavin.sharp)
Comment on attachment 543938 [details] [diff] [review]
alternate patch

I found a few issues:
- cookiesChildren reference to folder-item.png in browser/themes/winstripe/browser/preferences/preferences.css was not updated (to match gnomestripe)
- this looks like it makes chrome://global/skin/tree/item.png from pinstripe unused
- reference to folder-item.png in browser/themes/winstripe/browser/browser.css for .bookmark-item and #page-proxy-favicon were not updated

That last one is tricky - page-proxy-favicon has a special pageproxystate="invalid" styling on windows that we'd lose with the new image. Linux and Mac just use a different opacity, so perhaps we want to go with that here too?

Apart from those, this looks good.
Attachment #543938 - Flags: review-
(Assignee)

Comment 16

6 years ago
Created attachment 552562 [details] [diff] [review]
patch v2

Addressed comments. I want to test this on Windows to make sure it does the right things in places where I had to change around the list-style-image rules for folder items.
Assignee: shorlander → margaret.leibovic
Attachment #543196 - Attachment is obsolete: true
Attachment #543938 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #552562 - Flags: review?(gavin.sharp)
Comment on attachment 552562 [details] [diff] [review]
patch v2

The greyed-out icon (rect(32px, 16px, 48px, 0px)) in winstripe's chrome://global/skin/icons/folder-item.png is probably now unused. Probably worth a bug filed to remove it.
Attachment #552562 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 18

6 years ago
http://hg.mozilla.org/integration/fx-team/rev/a721c6686657
Status: ASSIGNED → NEW
Whiteboard: [fixed-in-fx-team]
(Assignee)

Updated

6 years ago
Depends on: 679024
(Assignee)

Comment 19

6 years ago
(In reply to Gavin Sharp from comment #17) 
> The greyed-out icon (rect(32px, 16px, 48px, 0px)) in winstripe's
> chrome://global/skin/icons/folder-item.png is probably now unused. Probably
> worth a bug filed to remove it.

Filed bug 679024.
Status: NEW → ASSIGNED
Comment on attachment 552562 [details] [diff] [review]
patch v2

http://hg.mozilla.org/mozilla-central/rev/a721c6686657
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
Bookmarks for sites that do not provide a favicon have this icon. This causes issues similar to Bug 580194
A better icon should be used.
Created attachment 557175 [details]
Favicon not visible on dark background
Depends on: 690195
(Reporter)

Updated

6 years ago
No longer depends on: 690195
Depends on: 685059

Comment 23

6 years ago
(In reply to sdrocking from comment #21)

> A better icon should be used.

+1

Why "fix" something that's not really broken?

A dotted/dashed outline usually implies a missing element. Favicons are not required for a webpage/website. Why is Firefox using an icon that implies an missing element for something that's not required?

Just stating my opinion as feedback against the new default icon are starting to come in from users after update to 8.0

Comment 24

6 years ago
(In reply to Dave from comment #23)

> A dotted/dashed outline usually implies a missing element. Favicons are not
> required for a webpage/website. Why is Firefox using an icon that implies an
> missing element for something that's not required?

+1

Even a change as simple as converting the dashed outline to a solid outline would be an improvement, so the icon would no longer be implying a missing element.
(In reply to Joshua Lawrence from comment #24)
> (In reply to Dave from comment #23)
> 
> > A dotted/dashed outline usually implies a missing element. Favicons are not
> > required for a webpage/website. Why is Firefox using an icon that implies an
> > missing element for something that's not required?
> 
> +1
> 
> Even a change as simple as converting the dashed outline to a solid outline
> would be an improvement, so the icon would no longer be implying a missing
> element.

Bug 685059 may be a better solution.

Comment 26

6 years ago
I'm reading from bug https://bugzilla.mozilla.org/show_bug.cgi?id=701287 where quite a few users are reporting missing favicons.  Favicons are missing from about 60% of webpages and I am getting the default dashed box now.  There are two webpages that can demonstrate the issue.
http://www.gardenweb.com/
http://forums.gardenweb.com/forums/legumes/

The first webpage has a green leaf showing up as the favicon.  The second had a green leaf until the change from firefox 7 to firefox 8.  The issue is that users are visually oriented and use the favicon to switch between tabs instead of reading the link name.  If you dig into the code, there is a rel=favicon statement that sets this up. The pages which are now missing the favicon were getting it from a reference statement.
(In reply to Darrel Jones from comment #26)
> http://www.gardenweb.com/
> http://forums.gardenweb.com/forums/legumes/
> 
> The first webpage has a green leaf showing up as the favicon.  The second
> had a green leaf until the change from firefox 7 to firefox 8.

The second web page doesn't display an icon for me, using Firefox 7.0.1 (same as in Firefox 8). If you can reproduce this issue, please do go ahead and file a new bug and CC me, and we can investigate further.

Updated

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