91881 - javascript error in cookieTasksOverlay.xul

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect)

Description

[Henrik Gemal]

Start the console and from within the console select:
Tasks -> Privacy -> Cookies -> Unblock from this site

you'll get:

Error: window._content has no properties
Source File: chrome://cookie/content/cookieTasksOverlay.xul
Line: 80

If you select Tasks -> Privacy -> Cookies -> Block from this site

You'll get:

Error: window._content has no properties
Source File: chrome://cookie/content/cookieTasksOverlay.xul
Line: 85

Comment 1

[Chase Tingley]

This looks to be a dup of bug 74541.  Henrik, I notice you filed both -- is this
something different?

Comment 2

[Stephen P. Morse]

Bug 74541 was a double bug referring to errors at lines 57 and 85.  Furthermore, 
it gave no steps to reproduce.  Henrik has since clarified that bug report with 
the necessary reproduce steps, but only for the line 57 bug (althought I cannot 
reproduce it using the steps he gave).  This report is for the line 85 bug.

This shows the importance of not combining two bugs in one report.  The line 57 
bug I am unable to reproduce and so have marked bug 74571 as works for me.  The 
line 80/85 bug as described here I can reproduce and am currently investigating 
and will eventually fix it and close this report out as fixed.  If this was all 
in one report, I would not know whether to close the report out as fixed or as 
works-for-me.

Comment 3

[Stephen P. Morse]

Oops, I meant 74541 above, not 74571.

Comment 4

[Stephen P. Morse]

Attachment: patch that removes errors at both lines 80 and 85

Comment 5

[Stephen P. Morse]

vishy, blake, please review.  thanks.

Comment 6

[Stephen P. Morse]

Actually a better thing to do is to remove the cookie/image items from the 
tasks->privacy menu when in the javascript console.  Attaching a patch that does 
that.

If we don't remove the items from the menu, but rather attempt to fix all the 
places where problems will occur when in the javascript console, we quickly get 
into trouble.  Specifically, we don't know whether or not to hide the 
imageblocking menu items since that is controlled by a pref and we can't test 
check the prefs when in the javascript console.

Comment 7

[Stephen P. Morse]

Attachment: patch that removes menu items rather than attempting to fix individual errors

Comment 8

[Stephen P. Morse]

Attachment: Patch that fixes bug 74541 as well

Comment 9

[Blake Ross]

I am still confused as to why we can't check the prefs when we open the Tasks menu.

Comment 10

[Viswanath Ramachandran]

I'm not convinced that we should make the tasks menu be different in the 
JavaScript Console (I'm not sure that why the menu is there to begin with in the 
JS console but thats a different question). I'd rather if we fixed the errors 
than removed the menu items. 

German - is there any invariant that the Tasks menu has to be same everywhere?

Comment 11

[Stephen P. Morse]

We can't fix the underlying problem as I already described.  We keep running 
into the following brick wall:

1. Image manager is to appear in commercial product if pref is set
2. Testing a pref works from the browser window but not the js console
3. Therefore browser window can test pref and determine if image manager
   menu items are to be displayed
4. Testing a pref does not work from js-console window
5. Therefore js-console window cannot determine whether or not it should
   display the image manager menu items.
6. Therefore it is not possible to have same taskmenu display in both 
   browser window and js-console window.

Comment 12

[Blake Ross]

I cannot find where you explained why we are unable to test the pref from the js
console.

Reading the original bug, I agree that the block/unblock items should be
removed, but not all of the items.

Comment 13

[Stephen P. Morse]

Problem is that this.pref doesn't exist when in js-console window.  So it 
requires additional code to obtain the pref.  Attaching new patch that does 
this.

But I still think that its wrong to have the ability to block cookies, bring up 
cookie tutorial, etc from the javascript console.  Taking it one step further, 
it's wrong to have any task menu there at all.  But assuming that we have to 
have it, then this is as good a patch as any.

Comment 14

[Stephen P. Morse]

Attachment: reverting back to patch that simply fixes error messages but keeps all menu items

Comment 15

[Viswanath Ramachandran]

r=vishy on this patch. 
I think we shd revisit why the Menu is there for JS console after m0.9.4, 
when eMojo has branched. 

Comment 16

[Blake Ross]

+      var pref;
+      pref = Components.classes['@mozilla.org/preferences;1'];
+      pref = pref.getService();
+      pref = pref.QueryInterface(Components.interfaces.nsIPref);
+      if (!pref.GetBoolPref("imageblocker.enabled")) {

I believe the new way is to use nsIPrefBranch.  Also, need to null check pref or
leave it in the try/catch...

+      if (!window._content) {

I think this will cause a strict warning (or maybe just an error).  How about if
("_content" in window)?

sr=blake

Comment 17

[Stephen P. Morse]

Attachment: patch with blake's suggestions

Comment 18

[Stephen P. Morse]

Fix checked in.

Comment 19

[Henrik Gemal]

hmmm did this really get checked in? I'm still seeing the js error...
build 20010811

Error: window._content has no properties
Source File: chrome://cookie/content/cookieTasksOverlay.xul
Line: 93

Error: window._content has no properties
Source File: chrome://cookie/content/cookieTasksOverlay.xul
Line: 103

Comment 20

[Stephen P. Morse]

Attachment: Making test more bulletproof

Comment 21

[Stephen P. Morse]

cc'ing blake and matt for reviews

Comment 22

[Blake Ross]

sr=blake

Comment 23

[matt]

r=matt

Comment 24

[Stephen P. Morse]

fix checked in

Comment 25

[Tom Everingham]

verified:

WinNT 4 2001-09-20-11-0.9.4
Mac os9 2001-09-20-12-0.9.4
Linux rh6 2001-09-20-17-0.9.4