Image manager shows 2 "top" tabs




Image Blocking
18 years ago
15 years ago


(Reporter: Chuck R., Assigned: Stephen P. Morse)



Firefox Tracking Flags

(Not tracked)




(4 attachments)



18 years ago
I have build 2000111404. When I went to Tasks, Privacy and Security, Image
Manager, View sites that can/not... the image manager appeared.

It has 3 tabs. The first and third which showed up as "top" (they were both
white) so I didn't know which tab was really showing.

Reproduce: 100%

Comment 1

18 years ago
Confirmed with 111508 mozilla trunk build on NT.  I'm also seeing it on today's
mac and linux builds. Updating Platform, OS and component
Assignee: asa → morse
Component: Browser-General → Cookies
Ever confirmed: true
OS: Windows 98 → All
QA Contact: doronr → tever
Hardware: PC → All

Comment 2

18 years ago
Now that's real strange.  There is javascript to select the third tab when the 
dialog is brought up as the image viewer.  But for some reason, the original 
first tab is also being selected.  Sounds like a regression somewhere in xul.

However we can easily side-step whatever xul problem there is by explicitly 
adding two lines that deselect the first tab.  Attaching patch.
Summary: Image manager shows 2 "top" tabs → [w]Image manager shows 2 "top" tabs

Comment 3

18 years ago
Created attachment 19277 [details] [diff] [review]
Patch to de-select the first tab

Comment 4

18 years ago
Looks good.  I noticed there's another CookieViewer.js file in

If that file is obsolete it should be removed.


Comment 5

18 years ago
Nevermind.  That other file is a placeholder.
You should never have to set the selected attributes yourself.

When the dialog loads, set the selectedTab property of the tabcontrol to be the
images tab or the cookies tab. This will ensure everything else happens

Comment 7

18 years ago
OK, that works.  Attaching new patch.

Comment 8

18 years ago
Created attachment 19363 [details] [diff] [review]
Revised patch

Comment 9

18 years ago
I thought that's what wasn't working in the first place.  Whatever.  r=dbragg  

Comment 10

18 years ago
What wasn't working in the first place was setting the "select" property on the 
<tab>.  The change that ben suggested (and that is in the latest patch) is to 
set the "selectTab" property on the <tabcontrol>.

Thanks for the review.
Is element2 declared outside this anywhere? (A peek in lxr showed it wasn't)...
you should replace

element2 = document.getElem...

with var element2 = document.getElem...

if not, to prevent js warnings (and prevent the variable from contaminating the
global scope).

Comment 12

18 years ago
Created attachment 19584 [details] [diff] [review]
revised patch to include "var"

Comment 13

18 years ago
New patch checked in.  dbragg, could you please re-review the new patch.  

Comment 14

18 years ago

Comment 15

18 years ago
Oops, my comment above about "new patch checked in" was in error.  I meant to 
say "new patch attached".  According to the rules, nothing can be checked in 
until it is re-reviewed and re-approved.

Now it has been re-reviewed.  Ben, please re-approve.  Thanks.

Comment 16

18 years ago
*** Bug 61261 has been marked as a duplicate of this bug. ***

Comment 17

18 years ago
Created attachment 19758 [details] [diff] [review]
Might as well fix the cookietab to use selectedTab while we're here

Comment 18

18 years ago
mkappy, I appreciate your wanting to help but I'm afraid it's going to backfire.  
The current bug report deals with the image manager tabs and that is what my 
patch is fixing.  If you want to fix the cookie manager as well (which currently 
isn't broken), please open a new bug on that one.  This bug has been in the 
review queue for 12 days now and if you change the patch then we will have to 
start the review process all over.


18 years ago
Summary: [w]Image manager shows 2 "top" tabs → Image manager shows 2 "top" tabs
Whiteboard: [w]

Comment 19

18 years ago
*** Bug 61261 has been marked as a duplicate of this bug. ***

Comment 20

18 years ago
My apologies for confusing the issue.

Based on Goodgers comment that the selected attribute should never be set 
explicitly, I thought it would be worthwhile to also fix the case where the 
cookietab is explicitly setting the selected attribute rather than setting 
selectedTab about 6 lines above this one.

Sometimes it's hard to get code fixes like this one in when they don't actually 
have a visual consequence, so I thought it might be worthwhile to fix it here 
while the code is open.

Again, sorry for the confusion.
Not every change to a patch requires re-review from the beginning.  If the
substance changes completely, or if most of the patch changes in a deep way
(e.g., not just tabs cleanups or comment changes), then of course.  Use your own

I agree with mkaply's suggestion. Since we're not in lockdown anymore, it
doesn't seem that it would hurt to make this change. 

Comment 23

18 years ago
Checked in a hybrid of kaply's patch and my patch.
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 24

18 years ago
*** Bug 56447 has been marked as a duplicate of this bug. ***


17 years ago
Whiteboard: [w]


15 years ago
Component: Cookies → Image Blocking
QA Contact: tever → nobody
You need to log in before you can comment on or make changes to this bug.