Last Comment Bug 675927 - [Mac] The Web Console Window freezes when drag and drop the parent tab to create a new window
: [Mac] The Web Console Window freezes when drag and drop the parent tab to cre...
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: x86 Mac OS X
: P2 critical (vote)
: Firefox 15
Assigned To: Thaddee Tyl [:espadrine]
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-02 07:15 PDT by AndreiD[QA]
Modified: 2012-05-29 11:49 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix bug 675927 (in mac) and 659775 (in all architectures). (585 bytes, text/plain)
2012-05-15 17:59 PDT, Thaddee Tyl [:espadrine]
no flags Details
Solve the bug (683 bytes, patch)
2012-05-17 14:45 PDT, Thaddee Tyl [:espadrine]
rcampbell: review+
mihai.sucan: review+
Details | Diff | Splinter Review

Description AndreiD[QA] 2011-08-02 07:15:36 PDT
Build ID:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0a1) Gecko/20110720 Firefox/8.0a1

Steps to reproduce:
1. Launch Firefox on a clean profile
2. Launch the Web console and set it in window mode (change its "position")
3. Drag and drop the parent tab of the webconsole to create a new window

Expected result:
3. The web console window is expected to remain functional

Actual result:
3. The web console window freezes 

*Notes:
Last good nightly: 2011-07-08
First bad nightly: 2011-07-09

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6e461b1419b8&tochange=97012a02db93
Comment 1 Rob Campbell [:rc] (:robcee) 2011-08-02 07:42:40 PDT
are you sure about that hg changeset? I don't see anything that would cause that kind of regression.
Comment 2 Rob Campbell [:rc] (:robcee) 2011-08-02 07:45:10 PDT
can I get a test for this on Windows as well? I suspect there are going to be issues with all platforms.
Comment 3 AndreiD[QA] 2011-08-02 07:54:56 PDT
I did the regression range with the mozregression tool which displayed that pushlog.
After a talk with msucan on IRC, he suggested bug 592469 as the culprit for this issue.

(In reply to comment #2)
> can I get a test for this on Windows as well? I suspect there are going to
> be issues with all platforms.

I will get back with a thorough tests on Windows and Linux regarding this issue. From a brief look it works on today's Nightly builds on:
Windows XP: Mozilla/5.0 (Windows NT 5.1; rv:8.0a1) Gecko/20110801 Firefox/8.0a1
Windows 7: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110801 Firefox/8.0a1
Ubuntu 11.04: Mozilla/5.0 (X11; Linux i686; rv:8.0a1) Gecko/20110802 Firefox/8.0a1
Comment 4 Mihai Sucan [:msucan] 2011-08-02 08:01:52 PDT
Bug 592469 *might* be the cause, but the problem is certainly a real edge case for which we had no test.

I have to check and see if 592469 is really to be blamed or not. From the given regression range only these patches are relevant in Web Console land - unless there's a change outside of the console code that triggers the issue.

The bug needs further investigation.
Comment 5 AndreiD[QA] 2011-08-03 00:41:55 PDT
(In reply to comment #1)
> are you sure about that hg changeset? I don't see anything that would cause
> that kind of regression.

As commented before, except for Mac, this works on all the other systems: Windows XP, 7x64, 8x86, Ubuntu 11.04 on the latest Nightly
Comment 6 Dave Camp (:dcamp) 2011-10-27 09:27:06 PDT
We're doing developer tool prioritization, filter on 'brontozaur'
to ignore the spam.
Comment 7 Thaddee Tyl [:espadrine] 2012-05-15 17:59:05 PDT
Created attachment 624262 [details]
Fix bug 675927 (in mac) and 659775 (in all architectures).

This patch also solves bug 659775[1]. Apparently, on Mac,
dragging a tab into its own window closes the tab
(and re-opens it in a new window), which made the
Web Console go insane.

  [1] "When the Web Console is in a separate window,
      Ctrl+W/Cmd+W should close it".
Comment 8 Mihai Sucan [:msucan] 2012-05-16 11:31:24 PDT
Comment on attachment 624262 [details]
Fix bug 675927 (in mac) and 659775 (in all architectures).

Review of attachment 624262 [details]:
-----------------------------------------------------------------

Hello Thaddee! Thanks for your patch!

Can you please explain the fix?

The console panel popup has code for the popuphidden event which calls deactivateHUDForContext() when hidePopup() is called. Why would you call hidePopup() again in deactivateHUDForContext()? Please see HUD_createOwnWindowPanel().
Comment 9 Thaddee Tyl [:espadrine] 2012-05-16 14:38:27 PDT
By testing the system, it appears to me
that the HUD_onPopupShown() function is not run (at least on mac),
while the HS_onTabClose(aEvent) function does run deactivateHUDForContext(),
which, with my patch applied, closes the panel popup.

Is line 3109 of webconsole/HUDService.jsm actually useful on other platforms?
(With my patch applied, that is.)
I don't seem to need it, on mac.
Comment 10 Mihai Sucan [:msucan] 2012-05-17 02:11:16 PDT
(In reply to Thaddee Tyl [:espadrine] from comment #9)
> By testing the system, it appears to me
> that the HUD_onPopupShown() function is not run (at least on mac),

Do you mean HUD_onPopupHidden?

> while the HS_onTabClose(aEvent) function does run deactivateHUDForContext(),
> which, with my patch applied, closes the panel popup.
> 
> Is line 3109 of webconsole/HUDService.jsm actually useful on other platforms?
> (With my patch applied, that is.)
> I don't seem to need it, on mac.

consoleWindowUnregisterOnHide works like a "semaphore" which tells if the Web Console should close when popuphidden happens, or if it should do nothing (when repositioning happens). This should be needed on all systems.

I will test your patch today or tomorrow on Mac and see how it works.
Comment 11 Thaddee Tyl [:espadrine] 2012-05-17 08:14:42 PDT
(In reply to Mihai Sucan [:msucan] from comment #10)
> Do you mean HUD_onPopupHidden?

Yes.

> consoleWindowUnregisterOnHide works like a "semaphore" which tells if the
> Web Console should close when popuphidden happens, or if it should do
> nothing (when repositioning happens). This should be needed on all systems.

On mac at least, popuphidden does not happen, which is the cause of the bug.

It is my understanding that on all platforms, tabclose happens,
which runs deactivateHUDForContext() anyway.
Comment 12 Mihai Sucan [:msucan] 2012-05-17 08:23:26 PDT
I tested on Mac OS 10.7.3 and without any patch it all works as expected - no freeze happens (I used a local fx-team build, hg pulled today). When I move the tab into its own window the Web Console closes correctly (as expected). The same happens with your patch applied.

Is this bug specific to some Mac OS version?
Comment 13 Mihai Sucan [:msucan] 2012-05-17 11:46:39 PDT
Tested again and with help from Rob I found the steps to reproduce the Web Console freeze on my Mac. Thanks Rob!

What I found is that, yes, indeed, the popuphidden event does not fire on Macs. I also tested on Linux and it doesn't fire the event on Linux either. The only difference is that on Linux and Windows if you remove the xul:panel from the document, the panel will close (as expected). On Mac there's a Gecko bug that freezes the panel when you remove the panel without first hiding it. So, please open a follow-up bug about this. Thanks!

I found that it is most appropriate to add hud.consolePanel.hidePopup() in unregisterDisplay() as follows:

  // Remove the HUDBox and the consolePanel if the Web Console is inside a
  // floating panel.
  if (hud.consolePanel && hud.consolePanel.parentNode) {
    hud.consolePanel.hidePopup();
    hud.consolePanel.parentNode.removeChild(hud.consolePanel);
    hud.consolePanel.removeAttribute("hudId");
    hud.consolePanel = null;
  }

This is the place where I believe the hidePopup() call fits - it's after hud.consoleWindowUnregisterOnHide = false; and it's where the console panel is removed.

Please update the patch accordingly. A try server push would probably be needed as well. I tested this on my system (on both Mac and Linux) - it seems to work well.

Thank you!
Comment 14 Thaddee Tyl [:espadrine] 2012-05-17 14:45:10 PDT
Created attachment 624908 [details] [diff] [review]
Solve the bug

Calling hidePopup() in unregisterDisplay().
Comment 15 Thaddee Tyl [:espadrine] 2012-05-17 16:21:45 PDT
I have submitted this bug at 756305 ([Mac] XUL panel freezes when you remove the panel).
Comment 16 Mihai Sucan [:msucan] 2012-05-18 01:27:39 PDT
Comment on attachment 624908 [details] [diff] [review]
Solve the bug

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

Thank you!
Comment 17 Rob Campbell [:rc] (:robcee) 2012-05-18 10:00:59 PDT
Comment on attachment 624908 [details] [diff] [review]
Solve the bug

delightful.
Comment 18 Rob Campbell [:rc] (:robcee) 2012-05-18 10:04:13 PDT
Comment on attachment 624262 [details]
Fix bug 675927 (in mac) and 659775 (in all architectures).

clearing flags.
Comment 19 Panos Astithas [:past] 2012-05-21 23:34:05 PDT
https://hg.mozilla.org/integration/fx-team/rev/e3702cde36ad
Comment 20 Tim Taubert [:ttaubert] 2012-05-22 06:23:19 PDT
https://hg.mozilla.org/mozilla-central/rev/e3702cde36ad
Comment 21 Paul Silaghi, QA [:pauly] 2012-05-28 08:28:22 PDT
Dragging and dropping the parent tab of the webconsole to create a new window makes the Web Console to close correctly, no more freeze issue. Verified fixed on FF 15.0a1 (2012-05-27) on Mac OS X 10.6.8.

One issue though. After drag and drop the tab in a new window and the web console closes, if switching back to the other FF window, in Tools/Web Developer I see the web console checked. What do you think?
Comment 22 Thaddee Tyl [:espadrine] 2012-05-29 11:49:29 PDT
Paul: this bug is more generic than that, and unrelated to bug 675927.

See <https://bugzilla.mozilla.org/show_bug.cgi?id=759384>.

Note You need to log in before you can comment on or make changes to this bug.