210520 - site icons in the bookmarks menu

Status: RESOLVED FIXED

(Camino Graveyard :: Bookmarks, defect)

Description

[S Woodside]

Display site icons in the bookmarks menu

Comment 1

[S Woodside]

Attachment: patch to enable site icons in the bookmarks menu

two lines, changed NO to YES

Comment 2

[Simon Fraser [no longer active]]

This can cause delays building the menu, iirc. Try it on a huge BM menu with
lots of icons in, and measure the impact on startup time.

Comment 3

[S Woodside]

how big is huge? I have 200, is that enough?

Comment 4

[S Woodside]

[posted to [e-mail] list]
We need someone to test out a special build of Camino to see if the bookmarks
menu can have the site icons displayed. Here:
    http://simonwoodside.com/camino-bookimg.tgz
The tester should have a lot of bookmarks, hundreds preferably, that have site
icons on them. Then this is the testing procedure.
1. Ensure all of the site icons for bookmarks are in the cache. You can tell if
they are displayed in the manage bookmarks pane. (and in this build, they'll be
in the menu too)
2. Quit all other applications
3. Launch Camino 5 times, and record the time it takes until the window appears
for each launch
4. Then, erase your cache. Now there should be no site icons in the bookmarks
pane now.
5. Repeat step 3
Then send back the 10 launch times. We'll use that result to decide if we can
enable this feature in the normal builds.

Comment 5

[S Woodside]

From: godard <godard@.hotmail.com>
Date: Sun Jun 29, 2003  10:40:57  PM America/Montreal
To: [e-mail]
Subject: Re: [Chimera] Re: testing request
Reply-To: [e-mail]

Hi Simon,

Are you casting dispersions upon my lowly powermac 7500? :-) (yes, that 
sadly slow 50mhz bus should be put into the equation of launch times)

There are exactly 113 bookmarks and approximately 60 have favicons.

All the best,
Gary

PS. For what my opinion is worth, yes, please implement this. Oh, and i 
had no unexpected crashes, snafu's, etc

S Woodside wrote:
> What kind of computer is that?
> 
> Also how many bookmarks did you have, and roughly how many of them had 
> site icons?
> 
> thanks,
> 
> simon
> 
> On Sunday, June 29, 2003, at 02:11  AM, godard wrote:
> 
>> Hi Simon,
>>
>> How's this?
>>
>> WITH ICONS (secs)
>> 10
>> 6
>> 7
>> 6
>> 7
>>
>>
>> WITHOUT ICONS (secs)
>> 7
>> 7
>> 8
>> 8
>> 7
>>
>> 7500 with 450/512 sonnet cpu, 288 ram, 10.2.6
>>
>> Gary

Comment 6

[S Woodside]

Testing results.

Jasper Houser [1] tested my build with this feature enabled. His times were,
with full cache, 4.6 s, and with empty cache, 4 s [2]. (15% longer) Gary [3]
tested with full cache, 7.4 s, with empty cache, 6.5 s. (14% longer) [2] Jasper
Houser had ~250 bookmarks ~25 with cached images. Gary had 113 bookmarks, ~60
with cached images.  [e-mail] and godard@.hotmail.com

[1] G4/533, 1.25GB RAM, OS X 10.2.6
[2] average over 5 consecutive launches
[3] Power Mac 7500 with G4/450/512 sonnet card, 288GB RAM, OS X 10.2.6

Back to you.

Comment 7

[Simon Fraser [no longer active]]

Ideally, we should lazily load the site icons, so that startup time is not
impacted at all. Also, what about memory use?

I'd suggest you key this off a UserDefaults pref, then at least people can tweak
it as they see fit.

Comment 8

[Chris Casciano]

see also related bugs:

Bug 182583
Bug 202054

Comment 9

[Mike Pinkerton (not reading bugmail)]

i'm with simon, let's do this as a hidden pref which we can turn on/off via
userDefaults and let people play. can you make another patch which does that? thx.

Comment 10

[S Woodside]

Attachment: patch with UserDefaults

defaults write org.mozilla.navigator BookmarksMenuImages YES

Comment 11

[David Haas (gone, not reading mail)]

work in bug 212630 handles this.

Comment 12

[Scott]

Lazy Loading sounds like a great idea.  Load the icons for a menu when that menu
is selected for viewing.

EXAMPLE>  If I click the bookmark menu and go down to the menu folder called
"Stuff", Mozilla would then load all the site icons for the items in the Stuff
area for the first level.  If there are submenus insie the Stuff menu, then thos
would not be loaded unless the user moved their mouse over those menus to view them.

All menus should be loaded only if the user leaves the mouse over the menu long
enough for it to invoke the view menu command or whatever command is generating
the submenu contents.

Comment 13

[Jasper]

Wouldn't that cause a lag in the performance of opening submenu's?

I have a private build from David that has this patch enabled and my main
concern on my machine (G4 533, 1Gb Ram) is the following. Whenever Camino is
launched cold and a new window is displayed, I get a spinning beach ball while
Camino is loading the icons, this causes a very awkward situation where Camino
isn't responding to anything you do.
I can emagine users won't like this at all, I sure don't, I have to wait a
second or two and then Camino turns back to normal.

I'd rather have a longer loading time (splash screen) or perhaps a sheet popping
up telling it's loading the icons, than having Camino not respond to anything at
all.

So please don't use this patch unless there is a way to get rid of all the
performance issues.

Comment 14

[Asa Dotzler [:asa]]

Comment on attachment 127305 [details] [diff] [review]
patch with UserDefaults

We have a new Camino request flag which can be set multiple times for a patch.
Moving old review requests to the new flag. Sorry for the spam.

Comment 15

[Mike Pinkerton (not reading bugmail)]

fixed by 212630