633196 - Object details don't open in the Web Console, when trying a second time

Status: VERIFIED FIXED

(DevTools :: General, defect)

Description

[Dan Wolff]

User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b11) Gecko/20100101 Firefox/4.0b11
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b11) Gecko/20100101 Firefox/4.0b11

When clicking on an object a window appears

Reproducible: Always

Steps to Reproduce:
1. Open the web console
2. Enter "window" on the command line (or the name of any JS object)
3. The web console responds with "[object Window]"
4. Click that line
5. A window appears - close it
6. Click the same line
Actual Results:  
Nothing happens.

Expected Results:  
The window should reappear with the object details should appear.

Comment 1

[Maniac Vlad Florin (:vladmaniac)]

WFM on 

Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110210 Firefox/4.0b12pre

Try creating a new profile see if that works for you 
http://support.mozilla.com/en-US/kb/Managing%20profiles

Comment 2

[Dan Wolff]

Same problem in a new profile. It seems like the bug only occurs when you close the window using the OS close button (not the close button at the bottom of the window), so step 5 needs to be modified.

Comment 3

[Maniac Vlad Florin (:vladmaniac)]

Well, this is another story then. This is clearly an OS integration issue with Linux. Guys?

Comment 4

[Rob Campbell [:rc] (:robcee)]

sounds likely. We don't have a close button in the title bar on object inspectors in other platforms.

Comment 5

[Maniac Vlad Florin (:vladmaniac)]

Low priority in my opinion

Comment 6

[David Dahl :ddahl]

(In reply to comment #2)
> Same problem in a new profile. It seems like the bug only occurs when you close
> the window using the OS close button (not the close button at the bottom of the
> window), so step 5 needs to be modified.

What linux distro and what theme are you using? My ubuntu Maverick does not have a close button on the pop-up window, so, this is WFM.

Comment 7

[Maniac Vlad Florin (:vladmaniac)]

David, I can repro this with an Ubuntu 10.04 (32bit)

Comment 8

[Dan Wolff]

I'm using Ubuntu 10.10 (maveric).
The close button doesn't appear when turning off all visual effects.

Comment 9

[David Dahl :ddahl]

(In reply to comment #8)
> I'm using Ubuntu 10.10 (maveric).
> The close button doesn't appear when turning off all visual effects.

Wow. this is probably a graphics bug. Good find. gonna kick it over there for evaluation.

Make sure to turn on visual effects in appearance.

Comment 10

[David Dahl :ddahl]

well - it may be a widget bug. hmm.

Comment 11

[Rob Campbell [:rc] (:robcee)]

yeah, I'm leaning towards widgets.

Comment 12

[Rob Campbell [:rc] (:robcee)]

mihai convinced me that we need to fix this in devtools code.

Comment 13

[David Dahl :ddahl]

(In reply to comment #12)
> mihai convinced me that we need to fix this in devtools code.

How is that possible? Compviz puts a close button on the panel. Whatever that does seems to be out of our scope. There are surely other panels affected by this - wouldn't it be better to have it fixed in widgets or elsewhere?

Comment 14

[Mihai Sucan [:msucan]]

Attachment: [checked-in] proposed fix

Proposed fix, as discussed with Robert on IRC. We are now not going to care if the window manager shows a Close button or not, in fact we ask for it to show, and we remove our Close button. This is technically a better solution on our side since window managers can always provide users with the ability to close a panel in other ways as well (eg. keyboard shortcuts).

Comment 15

[Mihai Sucan [:msucan]]

(In reply to comment #13)
> (In reply to comment #12)
> > mihai convinced me that we need to fix this in devtools code.
> 
> How is that possible? Compviz puts a close button on the panel. Whatever that
> does seems to be out of our scope. There are surely other panels affected by
> this - wouldn't it be better to have it fixed in widgets or elsewhere?

While you are correct, it is a problem in Widgets, but it's also a problem in our code. We should not rely solely on the use of our Close button for things to work as we want. We have to make sure that when the property panel is closed (no matter how), our code handles the situation properly.

Also, why doesn't the Network Panel suffer the same problem as the Object Inspector? It is because the implementation there relies on the popuphidden event - not on our own Close button. What I propose is simply that we do the same for the Object Inspector.

Comment 16

[Rob Campbell [:rc] (:robcee)]

he's pretty convincing, no?

Comment 17

[David Dahl :ddahl]

(In reply to comment #15)
> Also, why doesn't the Network Panel suffer the same problem as the Object
> Inspector? It is because the implementation there relies on the popuphidden
> event - not on our own Close button. What I propose is simply that we do the
> same for the Object Inspector.

I assumed they both had the same problem. Carry on.;)

Comment 18

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 511448 [details] [diff] [review]
[checked-in] proposed fix

acceptable!

Comment 19

[Mihai Sucan [:msucan]]

Comment on attachment 511448 [details] [diff] [review]
[checked-in] proposed fix

Thanks for the feedback+ Robert! Asking for review from Dolske.

Comment 20

[Justin Dolske [:Dolske]]

Comment on attachment 511448 [details] [diff] [review]
[checked-in] proposed fix

rs=me

Comment 21

[Mihai Sucan [:msucan]]

Thanks for the review+ and approval, Dolske!

Comment 22

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 511448 [details] [diff] [review]
[checked-in] proposed fix

http://hg.mozilla.org/mozilla-central/rev/f02cf259f317

Comment 23

[Rob Campbell [:rc] (:robcee)]

we neglected to remove the extra destroy call in inspector.js which relies on the PropertyPanel.jsm.

fix is:

diff --git a/browser/base/content/inspector.js b/browser/base/content/inspector.js
--- a/browser/base/content/inspector.js
+++ b/browser/base/content/inspector.js
@@ -795,17 +795,16 @@ var InspectorUI = {
 
     if (this.isStylePanelOpen) {
       this.stylePanel.hidePopup();
     }
     if (this.domPanel) {
       this.domPanel.hidePopup();
       this.domBox = null;
       this.domTreeView = null;
-      this.propertyPanel.destroy();
     }
     this.inspectCmd.setAttribute("checked", false);
     this.browser = this.win = null; // null out references to browser and window
     this.winID = null;
     this.selection = null;
     this.treeLoaded = false;
     this.closing = false;
     Services.obs.notifyObservers(null, "inspector-closed", null);

Comment 24

[Rob Campbell [:rc] (:robcee)]

checked in:

http://hg.mozilla.org/mozilla-central/rev/b6d820414fca

Comment 25

[Maniac Vlad Florin (:vladmaniac)]

Verified 

Mozilla/5.0 (Windows NT 6.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12