Closed
Bug 127349
Opened 23 years ago
Closed 23 years ago
Remove Partial Favicon support
Categories
(SeaMonkey :: UI Design, defect)
SeaMonkey
UI Design
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: tpringle, Assigned: bugzilla)
References
(Blocks 1 open bug)
Details
(Whiteboard: [adt3])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
bugs
:
review+
alecf
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
Summary: Remove partial support of favicons in the personal toolbar. Specifics:
Current state of Favicon support is as follows:
1) Favicons are picked up in the URL bar
2) Favicons are picked up in tabbed browser tabs
3) Favicons are picked up on a per session basis in the personal toolbar
4) Favicons are not picked up in bookmarks elsewhere
For the purposes of our next release we should remove the support for 3) above
because I believe this will appear broken to users and ultimately be annoying as
well. We should look at implementing items 3) and 4) at some later date.
Reporter | ||
Comment 1•23 years ago
|
||
Nominating nsbeta1 and plussing as per our earlier conversations. Let me know
if this is a problem.
Keywords: nsbeta1+
Updated•23 years ago
|
QA Contact: sairuh → claudius
Assignee | ||
Comment 3•23 years ago
|
||
Well, if we're sure we want to do this...
-> me
Assignee: ben → blaker
Assignee | ||
Comment 4•23 years ago
|
||
Comment 5•23 years ago
|
||
Attachment #72265 -
Flags: superreview+
Comment 6•23 years ago
|
||
Comment on attachment 72265 [details] [diff] [review]
patch
oh crap. I'll miss my partial favicons
r=hewitt
Attachment #72265 -
Flags: review+
Comment on attachment 72265 [details] [diff] [review]
patch
a=roc+moz for trunk
Attachment #72265 -
Flags: approval+
Assignee | ||
Comment 8•23 years ago
|
||
fixed.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 9•23 years ago
|
||
Why are these changes necessary? There already exist two prefs for favicons and
siteicons and currently all.js contains
pref("browser.chrome.site_icons", true);
pref("browser.chrome.favicons", false);
Isn't it possibly to disable site_icons in all.js, but still leave it alone for
those who care enough to manually re-enable them in prefs.js???
Assignee | ||
Comment 10•23 years ago
|
||
No...we're not removing favicon support from the places where it works (urlbar,
tabbrowser). We're removing it from the places that it does not work
consistently (the personal toolbar, the Bookmarks menu). That functionality is
too broken to bring up to release-level quality, so it should be disabled for
Mozilla 1.0.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Comment 11•23 years ago
|
||
You've removed the coolest thing added to Mozilla since POP3 over SSL :-(
Comment 12•23 years ago
|
||
I don't want to start a flamewar here, but just wanted to say thank you. Finally
NSCPs presidency over Mozilla comes in handy.
Comment 13•23 years ago
|
||
If you're gonna comment about how bad removing the support is, don't.
I'm sure there's already a bug filed for *fully functioning favicons in these
areas*. Find it and monitor it.
Don't spam here.
Comment 14•23 years ago
|
||
> I'm sure there's already a bug filed for *fully functioning favicons in these
> areas*.
I don't think so. There is only:
Bug 118070: "favicon of current site on personal toolbar changes when clicking
on another site in the toolbar" and
Bug 121030: "pref->apperance->show web site icons doesn't reset favicons in
personal toolbar until restart"
Both seem impossible to fix if there are no favicons in the personal toolbar to
test any patch :-(
I believe this feature can be only disabled by a default pref, something like:
pref("browser.chrome.favicons_in_personal_toolbar", false); not just removed.
Reopening, and let someone explain why I'm wrong.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 15•23 years ago
|
||
Blake already explained this, in Comment #10. We can add them back in once we
can do it properly, which won't be until after after the upcoming releases.
Please don't reopen this bug again, it won't do anyone any good. ->FIXED
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Comment 16•23 years ago
|
||
OK, bug 113574 now asks for favicons to be added back to
- bookmarks sidebar
- manage bookmarks menu
- personal toolbar
- toolbar bookmarks menu
I set bug 113574 to block all the bugs I could find that talk about issues with
favicons in those places. I also attached the patch reversing this one (e.g. to
add favicons back to toolbars) - hopefully that could go in early in 1.1 cycle
so that there is a chance things get fixed properly.
Comment 17•23 years ago
|
||
why couldn't this at least be a pref, that is off by default? Many of us love
our partial favicon support!
Comment 18•23 years ago
|
||
please please tell me I'm wrong but as I read and understand this bug, there
should be no favicons in the bookmark menus(toplevel and from the ptoolbar). In
my 2002030803 Win98 build they are most certainly there, even though they are
indeed absent from the Personal toolbar itself.
REOPENING
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 19•23 years ago
|
||
Gee, after I asked everyone nicely not to... ;-)
I also see this, using yesterday's build on Win2K.
Assignee | ||
Comment 20•23 years ago
|
||
Attachment #72265 -
Attachment is obsolete: true
Comment 21•23 years ago
|
||
Comment on attachment 73483 [details] [diff] [review]
rest of the fix (1 line)
sr=hewitt, but you should also remove validate="never"
Attachment #73483 -
Flags: superreview+
Comment 22•23 years ago
|
||
Comment 23•23 years ago
|
||
Comment on attachment 73483 [details] [diff] [review]
rest of the fix (1 line)
*sigh* for consistency's sake :)
r=alecf
I'm going to make an xpinstallable dynamic overlay one of these days that adds
all these attributes back. :)
Attachment #73483 -
Flags: review+
Comment 24•23 years ago
|
||
Comment on attachment 73483 [details] [diff] [review]
rest of the fix (1 line)
*sigh* for consistency's sake :)
r=alecf
I'm going to make an xpinstallable dynamic overlay one of these days that adds
all these attributes back. :)
Comment 26•23 years ago
|
||
re-nominating - there's a patch in hand with reviews already, and this is a
HIGHLY visible screw-up. The bookmarks menu for goodness sake!
Comment 27•23 years ago
|
||
Comment on attachment 73483 [details] [diff] [review]
rest of the fix (1 line)
a=brendan@mozilla.org for 1.0.
/be
Attachment #73483 -
Flags: approval+
Comment 28•23 years ago
|
||
Let's file this under 'Do the Right Thing', ->nsbeta1+/ADT3
Assignee | ||
Comment 29•23 years ago
|
||
fixed (again).
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 30•23 years ago
|
||
Reopening. I see favicons in my personal toolbar in the 2002032803 build.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 31•23 years ago
|
||
I don't see them in a nightly from today. The last commercial nightly I have (I
can't get another) is 3/26 and I don't see them there, either. Can others
comment on their findings?
Comment 32•23 years ago
|
||
No favicons in personal toolbar in build 2002032603 on Win2k.
Comment 33•23 years ago
|
||
ah! I see them - they're in bookmarks that are in subfolders in the personal toolbar
1) create a folder in your personal toolbar
2) visit a bugzilla query, and bookmark it in that folder
3) use the personal toolbar to visit that bugzilla query again
Reporter | ||
Comment 34•23 years ago
|
||
Alec, yep, I was just about to post a screenshot but crashed. It seems that it
is fixed except for this case.
Assignee | ||
Comment 35•23 years ago
|
||
Attachment #73483 -
Attachment is obsolete: true
Assignee | ||
Comment 36•23 years ago
|
||
the bug that never dies. can I get r/sr on this? (removes lots of attribute
cruft, the important changes are the two to <menuitem/>)
Comment 37•23 years ago
|
||
Comment on attachment 77413 [details] [diff] [review]
favicon's last stand.
yikes. sr=alecf
Attachment #77413 -
Flags: superreview+
Comment 38•23 years ago
|
||
Attachment #77413 -
Flags: review+
Comment 39•23 years ago
|
||
Comment on attachment 77413 [details] [diff] [review]
favicon's last stand.
a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #77413 -
Flags: approval+
Comment 41•23 years ago
|
||
Am I right assuming that the attachment 77413 [details] [diff] [review] will only go into the branch? When
will it be OK to put favicons back on the trunk (see bug 113574 for the patch)
so that we can try to figure out how to do them correctly?
Assignee | ||
Comment 42•23 years ago
|
||
fixed on branch and trunk.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Comment 43•23 years ago
|
||
adding fixed1.0.0 keyword (branch resolution). This bug has comments saying it
was fixed on the 1.0 branch and a bonsai checkin comment that agrees. To verify
the bug has been fixed on the 1.0 branch please replace the fixed1.0.0 keyword
with verified1.0.0.
Keywords: fixed1.0.0
Comment 44•23 years ago
|
||
The "put them back in" is now bug 143687.
Comment 45•22 years ago
|
||
favicons appear in tabs and the urlbar. they don't appear in the PT or bookmarks
menu (as expected with current implementation). tested with 2003.02.19 comm
trunk builds.
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: Core → Mozilla Application Suite
You need to log in
before you can comment on or make changes to this bug.
Description
•