Web console is buggy in window mode

RESOLVED FIXED in Firefox 10

Status

()

Firefox
Developer Tools
--
blocker
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: stechz, Assigned: rc)

Tracking

(Depends on: 1 bug, {verified-beta})

unspecified
Firefox 11
x86
Mac OS X
verified-beta
Points:
---

Firefox Tracking Flags

(firefox10+ verified)

Details

(Whiteboard: [qa+][qa!:10])

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

6 years ago
At least on Mac, when going to windowed mode, if I use the keyboard shortcut to close it (mac-shift-K) the console remains and then is completely unresponsive until I quit Firefox.

STR:
1. Type mac shift K
2. Change position -> window
3. Type mac shift K

Expected:
1. Window should close
2. Window becomes zombie-like
(Assignee)

Updated

6 years ago
Severity: normal → blocker
(Assignee)

Updated

6 years ago
Assignee: nobody → rcampbell
Status: NEW → ASSIGNED
(Assignee)

Updated

6 years ago
Depends on: 674589
(Assignee)

Comment 1

6 years ago
Created attachment 574722 [details] [diff] [review]
zombie killer

needs a test still, this could use a little round of feedback though.
Attachment #574722 - Flags: review?(dcamp)

Updated

6 years ago
Attachment #574722 - Flags: review?(dcamp) → feedback+
(Assignee)

Comment 2

6 years ago
also worth noting that the titlebar on the panel appears to be stealing focus. If you drag the panel to a new position and hit Cmd-K, nothing happens. This is surprising. If you focus the text area in (or any other browser widget) the toggle method gets fired. Probably another focus-related panel bug.
(Assignee)

Comment 3

6 years ago
Created attachment 574744 [details] [diff] [review]
zombie killer with test

updated
Attachment #574722 - Attachment is obsolete: true
Attachment #574744 - Flags: review?(dcamp)

Updated

6 years ago
Attachment #574744 - Flags: review?(dcamp) → review+
(Assignee)

Comment 4

6 years ago
https://hg.mozilla.org/integration/fx-team/rev/efd4925dc649
Whiteboard: [fixed-in-team]
(Assignee)

Comment 5

6 years ago
backout:

https://hg.mozilla.org/integration/fx-team/rev/a43d24581da0
Whiteboard: [fixed-in-team] → [backed-out][leaks]
(Assignee)

Comment 6

6 years ago
Created attachment 575283 [details] [diff] [review]
panel width adjustment

I think this'll help. Based on feedback from mihai in IRC.
Attachment #575283 - Flags: review?(mihai.sucan)
Comment on attachment 575283 [details] [diff] [review]
panel width adjustment

I am not sure if this is going to fix the memleak. Maybe go for a try push?
Attachment #575283 - Flags: review?(mihai.sucan) → review+
(Reporter)

Comment 8

6 years ago
Great work so far Rob :D
(Assignee)

Comment 9

6 years ago
No, I haven't forgotten about this. Thanks.
(Assignee)

Comment 10

6 years ago
Created attachment 578830 [details] [diff] [review]
zombie killer part deux

combined patches, fixed leak.

try results here: https://tbpl.mozilla.org/?tree=Try&rev=c407cf03b45b
Attachment #574744 - Attachment is obsolete: true
Attachment #575283 - Attachment is obsolete: true
Attachment #578830 - Flags: review?(mihai.sucan)
(Assignee)

Comment 11

6 years ago
Comment on attachment 578830 [details] [diff] [review]
zombie killer part deux

canceling review for one more little fix. Need to remove the consolePanel reference and node in positionConsole: to account for repositioning.
Attachment #578830 - Flags: review?(mihai.sucan)
(Assignee)

Comment 12

6 years ago
Created attachment 579395 [details] [diff] [review]
the return of the zombie killer

updated and refreshed. Now with 100% more zombie-killing awesomeness!
Attachment #578830 - Attachment is obsolete: true
Attachment #579395 - Flags: review?(mihai.sucan)
Comment on attachment 579395 [details] [diff] [review]
the return of the zombie killer

Review of attachment 579395 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch update. This looks better to me now. Just a couple of comments below...

::: browser/devtools/webconsole/HUDService.jsm
@@ -3335,5 @@
>  
> -      /*
> -       * Removed because of bug 674562
> -       * Services.prefs.setIntPref("devtools.webconsole.top", panel.panelBox.y);
> -       * Services.prefs.setIntPref("devtools.webconsole.left", panel.panelBox.x);

Probably we can revive all this now, just by changing from popuphidden to popuphiding.

(this would also probably save us from commenting-out parts of browser_webconsole_position_ui.js)

@@ +6193,4 @@
>  
>      if (hudRef && hud) {
>        if (hudRef.consolePanel) {
> +        hudRef.consolePanel.hidePopup();

Why this change here? This is the only one that doesn't seem to make sense for me. The web console won't close, it will only hide itself...
Attachment #579395 - Flags: review?(mihai.sucan) → review+
(Assignee)

Comment 14

6 years ago
hidePopup() calls deactivateHUDForContext if necessary.
(Assignee)

Comment 15

6 years ago
(In reply to Mihai Sucan [:msucan] from comment #13)
> Comment on attachment 579395 [details] [diff] [review] [diff] [details] [review]
> the return of the zombie killer
> 
> Review of attachment 579395 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> Thanks for the patch update. This looks better to me now. Just a couple of
> comments below...
> 
> ::: browser/devtools/webconsole/HUDService.jsm
> @@ -3335,5 @@
> >  
> > -      /*
> > -       * Removed because of bug 674562
> > -       * Services.prefs.setIntPref("devtools.webconsole.top", panel.panelBox.y);
> > -       * Services.prefs.setIntPref("devtools.webconsole.left", panel.panelBox.x);
> 
> Probably we can revive all this now, just by changing from popuphidden to
> popuphiding.

nope.

Error: panel.panelBox is undefined
Source File: resource:///modules/HUDService.jsm
Line: 3336

> (this would also probably save us from commenting-out parts of
> browser_webconsole_position_ui.js)

also, nope.

I think we're going to be better off landing this as-is. Further mucking around with the preferences setting in a panel that's being torn down isn't going to work.
(Assignee)

Comment 16

6 years ago
https://hg.mozilla.org/integration/fx-team/rev/bb8169ddc939
Whiteboard: [backed-out][leaks] → [fixed-in-fx-team][leaks]
(Assignee)

Updated

6 years ago
Whiteboard: [fixed-in-fx-team][leaks] → [fixed-in-fx-team]
(In reply to Rob Campbell [:rc] (robcee) from comment #15)
> (In reply to Mihai Sucan [:msucan] from comment #13)
> > Comment on attachment 579395 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > the return of the zombie killer
> > 
> > Review of attachment 579395 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]:
> > -----------------------------------------------------------------
> > 
> > Thanks for the patch update. This looks better to me now. Just a couple of
> > comments below...
> > 
> > ::: browser/devtools/webconsole/HUDService.jsm
> > @@ -3335,5 @@
> > >  
> > > -      /*
> > > -       * Removed because of bug 674562
> > > -       * Services.prefs.setIntPref("devtools.webconsole.top", panel.panelBox.y);
> > > -       * Services.prefs.setIntPref("devtools.webconsole.left", panel.panelBox.x);
> > 
> > Probably we can revive all this now, just by changing from popuphidden to
> > popuphiding.
> 
> nope.
> 
> Error: panel.panelBox is undefined
> Source File: resource:///modules/HUDService.jsm
> Line: 3336

Ah, ugly. We will switch some day to a real xul:window and no longer rely on the panel, anyway.
https://hg.mozilla.org/mozilla-central/rev/bb8169ddc939
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 11
(Assignee)

Comment 19

6 years ago
Comment on attachment 579395 [details] [diff] [review]
the return of the zombie killer

I'd like to get this into aurora. This fixes a critical bug where a user can end up with a zombie panel on their screen on OS X computers. It is impossible to close this panel without resorting to deep JavaScript juju.

Risk is relatively low-risk, has been tested on all platforms and comes with its own test.
Attachment #579395 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

6 years ago
tracking-firefox10: --- → ?

Updated

5 years ago
Attachment #579395 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 20

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/12211a96deaa
status-firefox10: --- → wontfix

Updated

5 years ago
status-firefox10: wontfix → fixed
Whiteboard: [qa+]

Comment 21

5 years ago
I have tried using the steps from the description on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0) Gecko/20100101 Firefox/10.0 beta 2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0) Gecko/20100101 Firefox/10.0 beta 2

The window is closing as expected on both OSes.

Setting resolution to Verified Fixed on Beta.
status-firefox10: fixed → verified
Keywords: verified-beta
Whiteboard: [qa+] → [qa+][qa!:10]

Updated

5 years ago
tracking-firefox10: ? → +
You need to log in before you can comment on or make changes to this bug.