Force shortcut desktop icons off on Windows XP

RESOLVED FIXED in Firefox 22

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: noozillander, Assigned: no52fear)

Tracking

({regression})

17 Branch
Firefox 24
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox21 affected, firefox22 verified, firefox23 verified, firefox24 verified)

Details

(Whiteboard: [mentor=bbondy][lang=c++])

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

7 years ago
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121010144125

Steps to reproduce:

Windows XP SP3. Updated FF from 16.0.1 to 17. All desktop icons with the familiar FF shortcut icons to websites now look like default windows system icons. No amount of fiddling/repairing brought back the familiar FF icons. Icons could be reset individually via Folder Options but this proved to be tedious and not sticky.


Actual results:

Gave up and reverted to FF 16.0.1. Icons ok again.
A number of people with the same problem have posted their concerns on the Firefox Support Forum, which seems to be invisible to the FF folks who might be able to fix this bug. See here: https://support.mozilla.org/en-US/questions/942316?page=2#answer-394644


Expected results:

FF shortcut icons should display consistently regardless of newer FF versions, as they used to up to 16.0.1

Updated

7 years ago
Group: core-security

Comment 1

7 years ago
same problem as bug 820123 affecting all FF/SeaMonkey versions with implemeted use of favicons on webpage shortcuts in Windows under Windows XP.

The ability to read PNG images from ICO and CUR format images was introduced in Windows Vista, Windows XP can't cope with such files and replace the icon with the default windows system icon.

Even if the problem will be solved somehow, there should be option in about:config to disable such questionable feature like suggested in bug 827784
Blocks: 110894
Component: Untriaged → Shell Integration
It's also confirmed on the French support forum: http://forums.mozfr.org/viewtopic.php?f=5&t=112636 and http://forums.mozfr.org/viewtopic.php?f=5&t=111095.

Can you try Firefox 21 Beta (http://www.mozilla.org/firefox/beta/) that contains the fix of bug 827784 and set browser.shell.shortcutFavicons to false in about:config?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(noozillander)
Keywords: regression
Summary: Since updating to Firefox 17, shortcut desktop icons have reverted to generic windows icons → shortcut desktop icons have reverted to generic windows icons on Windows XP
Version: 16 Branch → 17 Branch

Comment 3

6 years ago
I just installed FF 21.0, added the settings to config:about and it work. We see the nice FF icon again. Too bad the options is not predefined in about:config with true state, so it can be just switched to false.
Summary: shortcut desktop icons have reverted to generic windows icons on Windows XP → Force shortcut desktop icons off on Windows XP
Mentored info:
We want to change this condition to always enter when the user is using anything less than Vista:

http://dxr.mozilla.org/mozilla-central/widget/windows/nsDataObj.cpp#l1117

The check for OS version should be done like this:

if (... || !IsVistaOrLater()) {
Whiteboard: [mentor=bbondy][lang=c++]
(In reply to smz from comment #53)
> Initially I was about posting this in
> https://bugzilla.mozilla.org/show_bug.cgi?id=827784, but as it is now
> flagged as "fixed" and this "bug" seems to be the active one about issues
> related to the shortcuts-with-favicon issue, I'm posting here hoping to be
> not totally "off topic".
> 
> Now, sorry, I don't want to step on anyone's toes, but I would set it as the
> default on any OS.
> 
> Actually I can't talk about "any OS", but while it is a given fact that it
> is a real bug to try to use the "favicon shortcut" on WinXP, I find it
> particularly ugly and bad implemented on Win7 as well.
> 
> I'll try to justify such a "strong" wording and please keep in mind that
> while some absolute measures in the following analysis may differ according
> to your setting, the relative ones should not:
> 
> Point 1:
> With Win7 set up to show "small icons" (I'm allowed to do it, I can see
> them, so I think it is kosher for me to do it):
> 
> - A FF shortcut is displayed as 37x37 pixels white square.
> - Within it I have a 13x13 "favicon"
> - So on each side of the favicon I have a 12 pixels "border"
> - The area covered by the favicon is about 12.3% of the square shortcut area
> - I find this to be bad "real estate management"
> 
> Point 2:
> Shortcuts favicons are stored "off line"(pardon the inaccurate term) in
> /%userprofile%/AppData/..../shortcutCache and not "in line" (somehow
> serialized) within the shortcut itself.
> Whenever I copy/move/send my shortcut out of the domain of my user account,
> I am copying/moving/sending an "half baked" shortcut, castrated of much of
> its meaning.
> 
> Point 3:
> When I delete a FF shortcut, the corresponding .ico file in
> /%userprofile%/AppData/..../shortcutCache is not removed, potentially
> leaving behind a lot of garbage in my profile
> 
> So, this is what I think: *IF* we want favicons in shortcut's icons (I can
> trace a request for this to
> https://bugzilla.mozilla.org/show_bug.cgi?id=110894 reported on 2001-11-19,
> and I’m not "a priori" against that), well, please, let us implement it in
> "nice" and "correct" way, otherwise let's go back to the generic FF icon.
> 
> Let me also state this: I can't talk "about others" in a general way, but
> all the people I've talked to really do *hate* the new "tiny favicon"
> shortcuts, so I don't think I'm part of minority of "nuts" (but maybe I’m
> wrong...)
> 
> My apologies to everybody if I’ve been to strong with my words: English is
> not my mother tongue so I may have used impolite wording.
Personally I'd be fine with changing the default everywhere, unfortunately I'm not sure how to measure user perception for this feature.  I'm leaning towards just disabling it everywhere though by default.

Comment 7

6 years ago
Brian,

I agree with you: as it is now it would be much better to turn off the shortcuts-with-favicons feature by default.

I don't think many like to have their FF profile littered by dead .ico files and/or have internet shortcuts with a generic icon when moved to a different system or when the profile is cleaned/changed.

It will be different if icons were stored the same way that IE does (used to do?) when it create .URL files, that is storing the icon in the :favicon:$DATA alternate data stream of the .URL file.

Alternate data streams have their shortcomings too: when you move such an .URL files through a files system that doesn't support ADS (e.g. FAT32) you loose your icon but you are warned about the loss and the .URL file revert to its default icon, based on your browser choice (generic FF icon in our case)

Of course all of the above is true in the Windows platform and things may be (a lot) different in other cases.

It is also not clear to me what the implications are for Win8. We are anyway talking about .url files and not .website files, so *I think* there should be a total compatibility.

About your comment about user perception, yes, I understand it is now difficult to measure it, but... was that a concern when the shortcuts-with-icons were implemented?

I think that a fair compromise now would be to:

1. turn them off under WinXP
2. Let them on under Vista and above
3. provide a *prominent* option to turn them off
4. re-implement icon storage using ADS(under Windows, of course), possibly with an up-scaling of the original favicon (as IE does, if I'm not mistaken)

Points 1 to 3 should be addressed ASAP, possibly with the next minor *fix* release.
Flags: needinfo?(noozillander)

Comment 8

6 years ago
... it could be the other way round too: off everywhere, prominent option to have them back on again if someone so desires...
We could attempt to go with that plan but if we added an option in step #3 then we cannot request landing on aurora and beta (due to localized strings cannot be uplifted).  Step #4 would be its own separate bug.  

I think maybe the best thing to do is to just disable the pref in this bug, land it and uplift it to aurora and beta, and then post another bug for changes we want from there, such as ADS and perhaps UI in preferences.  If it got approval it could land on release 2013-06-25.
About the checkbox UI please also read this to see if you want one:
http://limi.net/checkboxes-that-kill (again UI changes can only go live on release by 2013-09-17 due to string changes).

Comment 12

6 years ago
Brian,

what do you think about this (revised) path:

1. have browser.shell.shortcutFavicons exposed in about:config
2. have it set to "false" if windows version <= WinXP, otherwise set to "true"

the above two points to be implemented ASAP, possibly for the 2013-06-25 release

3. open up discussion and see if it should be set to "false" by default on every platform.
4. give more visibility to the option
5. have a proper implementation for .url shortcuts, in line with what IE does.

the above three points to be addressed at a second time.

I understand the rationale for requesting to open "new bugs" for points 3 to 5, but aren't we going to loose "audience" interested in this topic? At the very list we should "advertise" the existences of the new bugs to everyone who subscribed to this and all other related bugs.

To those who think that this issue have "already been fixed", I would like to underline that, yes, I can consider it fixed too, but only from a personal (I mean "egoistical") point of view.

P.S.: Brian, I just noticed that while I was writing this comment you posted a link to an interesting article. I will read it later on, thank-you!!

Comment 13

6 years ago
above, please, correct "very list" with "very least"! :-)
> but aren't we going to loose "audience" interested in this topic?

No we simply copy the CC list in the new bug.
We don't keep bugs open that are already landed for various reasons, but one of them is because it makes it confusing when there is a need to uplift past the nightly builds. For example there are tracking flags and flags for verification, and it would be confusing which one it applied to.

Mentored info: See Comment 4. Also I forgot to mention that to expose the pref just add the pef's default value to browser\app\profile.

Comment 15

6 years ago
Fantastic!

... but now, who can actually implement those two small fixes? I'd love to be able to do it but unfortunately I'm not, and if we want that implemented "real soon" we probably need somebody with a good experience with "rapid repair" fixes.

As far as regards point 3 to 5, Brian, can you open the "new bug(s)": your English is definitely better than mine and I think it will be easier for you to express the issues at hand with a better wording...

I've read the article you posted and I totally agree with it! But if something has been broken in the past we should try to fix it now, don't you think?
I reached out to someone to see if they can help with a fix, if not I'll try a couple other people, if not I can pick it up myself. Yup let's get this bug done then I can post the next steps.

Comment 17

6 years ago
Great! Thanks!

I see this bug has 8 users in the CC list, while the CC list for the 100% related https://bugzilla.mozilla.org/show_bug.cgi?id=827784 has 17. Wouldn't it be better to put the missing ones in this bug CC list?

Also, as a personal note/request, where I can learn how to properly format posts within bugzilla (e.g. like the way you do for reference to other comments)? Am I allowed to use just plain HTML?
(In reply to smz from comment #17)
> Great! Thanks!
> 
> I see this bug has 8 users in the CC list, while the CC list for the 100%
> related https://bugzilla.mozilla.org/show_bug.cgi?id=827784 has 17. Wouldn't
> it be better to put the missing ones in this bug CC list?

They have seen the dependent bug so they can re-add themselves, but when we spin off other bugs from this we'll copy.

> Also, as a personal note/request, where I can learn how to properly format
> posts within bugzilla (e.g. like the way you do for reference to other
> comments)? Am I allowed to use just plain HTML?

Probably there is some documentation but it's really easy, all you do is put the word bug followed by an ID and it will make it a link.  If you want a comment link you just put bug followed by an ID followed by comment followed by an ID.  Normal URLs are auto formatted.

No HTML or other formatting will be interpreted.

Comment 19

6 years ago
Thanks again, Brian!

Sergio
Assignee

Updated

6 years ago
Assignee: nobody → no52fear
Assignee

Comment 20

6 years ago
Posted patch Bug 828284 (obsolete) — Splinter Review
Added patch for the first 2 requirements in Comment 12.
Attachment #754824 - Flags: review?(netzen)
smz I also posted this which should improve the situation for post XP:
bug 876700
Comment on attachment 754824 [details] [diff] [review]
Bug 828284

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

Looks great, thanks!
After you re-attach I'll post to try for tests and then I'll land it to mozilla-central Nightly builds for you :)

::: widget/windows/nsDataObj.cpp
@@ +1114,5 @@
>  
>    const char *shortcutFormatStr;
>    int totalLen;
>    nsCString path;
> +  if (!Preferences::GetBool(kShellIconPref, true) || WinUtils::GetWindowsVersion() < WinUtils::VISTA_VERSION) {

Could you split this into 2 lines? Most of the code tries to stay within a max line width of 80 chars:
if (!Preferences::GetBool(kShellIconPref, true) ||
    WinUtils::GetWindowsVersion() < WinUtils::VISTA_VERSION) {
Comment on attachment 754824 [details] [diff] [review]
Bug 828284

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

Mark the next patch with review+ by the way.
Attachment #754824 - Flags: review?(netzen) → review+
Assignee

Comment 24

6 years ago
Posted patch Bug 828284 (obsolete) — Splinter Review
Applied the suggested change.
Attachment #754824 - Attachment is obsolete: true
Attachment #755102 - Flags: review+
Assignee

Comment 25

6 years ago
Posted patch Bug 828284Splinter Review
Had a unnecessary tab in the previous patch.
Attachment #755102 - Attachment is obsolete: true
Attachment #755107 - Flags: review+
Thanks for the patch Adri!
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb83343d0a26
Target Milestone: --- → Firefox 24
https://hg.mozilla.org/mozilla-central/rev/cb83343d0a26
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Comment on attachment 755107 [details] [diff] [review]
Bug 828284

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 110894
User impact if declined: Users on Windows XP that create website shortcuts will get a default system icon for those shortcuts instead of a firefox icon.
Testing completed (on m-c, etc.): This is on m-c now, will verify before uplifting
Risk to taking this patch (and alternatives if risky): Very low
String or IDL/UUID changes made by this patch: None
Attachment #755107 - Flags: approval-mozilla-beta?
Attachment #755107 - Flags: approval-mozilla-aurora?
Comment on attachment 755107 [details] [diff] [review]
Bug 828284

Can't imagine this causing critical regressions post-release; approving for Aurora/Beta.
Attachment #755107 - Flags: approval-mozilla-beta?
Attachment #755107 - Flags: approval-mozilla-beta+
Attachment #755107 - Flags: approval-mozilla-aurora?
Attachment #755107 - Flags: approval-mozilla-aurora+
Verified as fixed on Firefox 22 beta 4 (and Windows XP SP 3):
Mozilla/5.0 (Windows NT 5.1; rv:22.0) Gecko/20100101 Firefox/22.0
Build ID: 20130605070403
Duplicate of this bug: 820123

Updated

6 years ago
Duplicate of this bug: 891893
Verified as fixed on Firefox 23 beta 8:
Mozilla/5.0 (Windows NT 5.1; rv:23.0) Gecko/20100101 Firefox/23.0
Build ID: 20130722172257
QA Contact: simona.marcu
Verified as fixed on Firefox 24 beta 10:
Mozilla/5.0 (Windows NT 5.1; rv:24.0) Gecko/20100101 Firefox/24.0
Build ID: 20130909203154
You need to log in before you can comment on or make changes to this bug.