146048 - image context menu is slow when image blocking is active

Status: VERIFIED DUPLICATE of bug 115818

(Core :: Graphics: Image Blocking, defect)

Description

[Tomi Leppikangas]

From bug #77051

When there is lot of sites listed in cookie or image block list, image
contextmenu is slow. I have 8 image block sites and about 500 cookie
block sites and after right clicking on image opening popup takes
about 2secs.

Steps to reproduse:

1. get lots of sites to image or cookie blockin list (maybe some scripts
   that adds sites to profile/cookperm.txt)
2. right click on image

Actual results:
  Takes about 2-3sec to open menu.

Expected:
  Menu opens instantly.




I hacked patch that shows the problem:
extensions/cookie/resources/content/cookieContextOverlay.xul:
      initImageBlocking : function () {
        try {
        // Block image depends on whether an image was clicked on, and,
        // whether the user pref is enabled.

/***********************************************************************
        gContextMenu.showItem
          ("context-blockimage",
           gContextMenu.onImage && cookieContextMenu.isPrefSet() &&
           cookieContextMenu.isBlockingImages());

        gContextMenu.showItem
          ("context-unblockimage",
           gContextMenu.onImage && cookieContextMenu.isPrefSet() &&
           !cookieContextMenu.isBlockingImages());
********************************************************************/
        } catch (e) {}
      },

When that code is commented out menus works instantly.

Comment 1

[Tomi Leppikangas]

When comparing how image blocking is handled in
cookieNavigatorOverlay.xul and cookieContextOverlay.xul
there seems to be some differencies.

cookieNavigatorOverlay.xul uses function
permissionmanager.testForBlocking(window._content.location,IMAGEPERMISSION)
and cookieContextOverlay.xul travels list line by line.

Can cookieContextOverlay.xul use that testForBlocking() function too?

Comment 2

[Tomi Leppikangas]

Attachment: patch that uses testForBlocking() for site search

Comment 3

[Stephen P. Morse]

Comment on attachment 84679 [details] [diff] [review]
patch that uses testForBlocking() for site search

looks good to me.
r=morse

Comment 4

[Sander]

Having well over 5000 image domains blocked (I generated a list of akamai
domains) :) contextmenu on images takes over 10 seconds to appear.
Going to apply the patch myself in a moment to test a bit, but I'm feeling
hopeful. uhm, nominating for mozilla 1.0 is not much use anymore, but should we
then nominate for mozilla 1.0.1?

Comment 5

[Sander]

*drools and applies the patch to all his builds*
Works like a dream. Thank you! :)

I did notice you used tabs in the patch though. Please replace those by spaces.

Comment 6

[Chris Waterson]

Comment on attachment 84679 [details] [diff] [review]
patch that uses testForBlocking() for site search

sr=waterson

Comment 7

[Tomi Leppikangas]

Morse, can you check this in? I dont have cvs access. Change those
tabs to spaces before checkin.

Comment 8

[Stephen P. Morse]

OK, checked in on trunk.

Comment 9

[Jeremy M. Dolan]

Awww... and I was *so* enjoying the nice, stable 1.0 branch. Haven't crashed in
a long long time. Now I have enough reason to use trunk again. *sigh*. But thanks.

Comment 10

[Tomi Leppikangas]

Verified fixed on linux nightly build 2002052507

Comment 11

[Sander]

Any chance we could get this in the branch as well? Seems pretty low risk to me...

Comment 12

[Stephen P. Morse]

Actually this is a dup of bug 115818, and the patches in both these reports are 
nearly identical.  However the patch in 115818 is slightly better in that it:

- has an added check for a null vector, which is a good thing to have

- has a more efficient return.  Instead of:

     if (X) return false;
     return true;

  it does simply

     return !X;

Comment 13

[Stephen P. Morse]

This patch has now been backed out of the trunk and the better patch in bug 
115818 has been put onto the trunk instead.

Comment 14

[Stephen P. Morse]

reopening so I can mark this as a dup

Comment 15

[Stephen P. Morse]

*** This bug has been marked as a duplicate of 115818 ***

Comment 16

[Tomi Leppikangas]

Verified as dup. Sorry that i didnt see other bug, patch in that
is better.