115818 - Context menu slow to appear over image

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

Right clicking on an image takes approximately 1-2 seconds for the context menu
to appear whereas right clicking anywhere else loads the context menu
instantaneously.

I've seen this problem for a few months and couldn't find anything in Bugzilla
about it.

Currently using Linux 2001121708.

Comment 1

[Diego Biurrun]

I see no speed difference, caillon.  Tried a fresh profile?  Linux 2001121606

Comment 2

[Diego Biurrun]

Then again, it does appear somewhat slower over images, though not as much as
you say.  I have a K6-2 500 512MB RAM, what do you run on?

Comment 3

[:deanis74]

Could this be due to the large number of items in the context menu?

Comment 4

[Boris Zbarsky [:bzbarsky]]

This is not blake's

Comment 5

[Alfonso Martinez]

changing OS -> All
I've also seen this behaviour in win98 for some time (maybe around 2 months) and
I thought that it was already reported.
In my P2-350Mh 128Mb it can take easily more that 3 or 4 seconds to show the
context menu on images, so I don't use it anymore.
Build 2001-12-17-03, win98

Comment 6

[Blake Ross]

-> law

Comment 7

[Boris Zbarsky [:bzbarsky]]

Does this happen on all images?  Only some images?

Comment 8

[Christopher Aillon (sabbatical, not receiving bugmail)]

Over every image.  I am beginning to speculate that this is related to the
number of cookies in the Allow/Deny list... I have cookies set to ask me when a
site requests to set a cookie.  I recently cleared my whole list and noticed a
speed improvement to match what happens when right clicking elsewhere.  It's
slowly deteriorated again as I re-add more cookie sites to the list.  I don't
have much proof of this yet since my list is still considerably small and the
pause is not as great as when I initially reported this....

Comment 9

[Diego Biurrun]

One more reason to finally implement a good cookie management which allows you
to allow a few sites manually and block the rest...  Go place some votes on bug
75915.

Comment 10

[Boris Zbarsky [:bzbarsky]]

That is in fact almost certainly it.  See the code at

http://lxr.mozilla.org/seamonkey/source/extensions/cookie/resources/content/cookieContextOverlay.xul#43

Comment 11

[Bill Law]

Sending to Steve.

Maybe we could make the image-block items hang off a submenu and then only pay
the price when the user wants to.  I know cascaded context menus are probably
heretical, though.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Maybe we should fix the API to allow asking for image permissions _only_?

Comment 13

[Boris Zbarsky [:bzbarsky]]

*** Bug 132648 has been marked as a duplicate of this bug. ***

Comment 14

[Ari]

I think that dividing image and cookie permission apis would only make this
faster by some small constant factor because someone could easily have many
image permissions in addition to many cookie blocking permissions.

In my opinion, the underlying problem is that we are getting an enumeration of
all the permissions and iterating through them in js. To speed this up, couldn't
we just use nsIPermissionManager::TestForBlocking() instead? TestForBlocking()
is implemented in nsPermissions through a similar O(n) lookup but at least it's
in native code. Furthermore, if we really wanted to make this fast, we could
reimplement TestForBlocking using a hashtable.

Comment 15

[Ari]

Attachment: patch to switch to TestForBlocking()

Switching to TestForBlocking() seems to make a big difference on its own.

Comment 16

[Ari]

Can someone take a look at my minimal patch? It seems to be something that could
be added for 1.0 without much risk.

Comment 17

[Boris Zbarsky [:bzbarsky]]

The patch looks reasonable to me, but Stephen Morse is the only one who can
really review it usefully.   Have you tried sending him email (not bugmail)
asking for a review?

Comment 18

[Stephen P. Morse]

Look reasonable to me too.
r=morse.

Comment 19

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 79775 [details] [diff] [review]
patch to switch to TestForBlocking()

adding r=morse from comment 18.

Comment 20

[Christopher Aillon (sabbatical, not receiving bugmail)]

Blake, jag, can either of you have a look at sr=ing this diff -uw for improving
perf of the image context menu?

Comment 21

[Athman Boukhaoua]

This is a perfect patch that's been available for over a month now.
It still works flawlessly in RC3, so there's not even a regression.

Why hasn't this patch already been checked in?

Comment 22

[Boris Zbarsky [:bzbarsky]]

This was checked in on the trunk at 05/24/2002 16:49.  See the checkin log.

Comment 23

[Stephen P. Morse]

Actually what was checked in was the patch in bug 146048, which I now realize is 
a dup of this bug.  And, in fact, the patches are almost identical, except for 
some added bulletproofing and a more efficient return in this one.  So I'd like 
to change what was checked in to be this patch instead.  If I can get an sr on 
it, I'll do that.

Comment 24

[Chris Waterson]

Comment on attachment 79775 [details] [diff] [review]
patch to switch to TestForBlocking()

sr=waterson

Comment 25

[Stephen P. Morse]

*** Bug 146048 has been marked as a duplicate of this bug. ***

Comment 26

[Stephen P. Morse]

Patch for bug 146048 has been backed out of the trunk and this patch was just 
checked in instead.

Comment 27

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 79775 [details] [diff] [review]
patch to switch to TestForBlocking()

Approved for 1.0.1 branch.

Please check into the 1.0.1 branch ASAP. once landed remove the
mozilla1.0.1+ keyword and add the fixed1.0.1 keyword

Comment 28

[Olivier Cahagne]

*** Bug 150836 has been marked as a duplicate of this bug. ***

Comment 29

[brightice]

Just wanted to say that on HP-UX 11.0 the time difference between opening an
image context menu and opening a 'normal' context menu was about 10(!) seconds.

Build 2002060922 is fine.

Comment 30

[sairuh (rarely reading bugmail)]

vrfy'd fixed using 2002.06.17.0x comm branch bits on linux rh7.2, win2k and mac
10.1.5.

Comment 31

[Matthias Versen [:Matti]]

*** Bug 156954 has been marked as a duplicate of this bug. ***