Closed Bug 648668 Opened 13 years ago Closed 13 years ago

update blank document default favicon

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 8

People

(Reporter: fryn, Assigned: Margaret)

References

Details

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

Attachments

(4 files, 2 obsolete files)

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*
(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.
Is this landed in the ux branch?
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
Oh ya, earlier it was backed out , thanks :)
Attached patch Update Blank Favicon - 01 (obsolete) — Splinter Review
Changes the default page favicon to a dotted outline
Attachment #543196 - Flags: review?(gavin.sharp)
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.
Attached patch alternate patch (obsolete) — Splinter Review
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-
Attached patch patch v2Splinter Review
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+
http://hg.mozilla.org/integration/fx-team/rev/a721c6686657
Status: ASSIGNED → NEW
Whiteboard: [fixed-in-fx-team]
Depends on: 679024
(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
Status: ASSIGNED → RESOLVED
Closed: 13 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.
Depends on: 690195
No longer depends on: 690195
(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
(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.
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.
Depends on: 702730
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: