Holding F12 breaks devtools toolbox and it can't be closed

VERIFIED FIXED in Firefox 52



Developer Tools: Framework
8 months ago
6 months ago


(Reporter: arni2033, Assigned: ochameau)


(Blocks: 1 bug, {regression})

27 Branch
Firefox 53

Firefox Tracking Flags

(firefox50 wontfix, firefox51 wontfix, firefox52 verified, firefox53 verified)


MozReview Requests


Submitter Diff Changes Open Issues Last Updated
Error loading review requests:


(1 attachment)



8 months ago
>>>   My Info:   Win7_64, Nightly 52, 32bit, ID 20161028030204 (2016-10-28)
1. Open url   data:text/html,<body></body>
2. Hold F12 for 3 seconds
3. Click close button in devtools toolbox

1. Open url   data:text/html,<body></body>
2. Execute the following code in scratchpad in browser context:
   function T(){try{gDevToolsBrowser.toggleToolboxCommand(gBrowser);}catch(e){}}T();setTimeout(T,100);
3. Click close button in devtools toolbox

AR:  Devtools toolbox breaks and can't be closed
ER:  Devtools toolbox should close

This is regression. So far Alexandre Poirot doesn't care about quality at all. Hopefully he'll
pay attention to this bug and fix it ASAP. If you do care about quality, please talk to him.

Wording:  (added, because some people may misunderstand)
1) "Alexandre Poirot doesn't care about quality at all" - that is true. Explanation:
   For any person, tracking new bugs (especially serious ones) that are set as blocking for bugs he
   implemented is a trivial task. If a person implements "improvement" like bug 1161072 (and therefore
   hang in bug 1197789, that was present for ONE YEAR with zero activity), that can only mean A or B
   A) he was aware of bug 1197789 => he intentionally added hang in browser (by keeping patch from
      bug 1161072 in browser) => therefore he doesn't care about quality at all.
   B) he wasn't aware of that bug => he didn't even care tracking whether he broke something =>
      had no intention of creating reliable product.
2) `"improvement"` - this expression contains quotes, because bug 1161072 only added hang.


8 months ago
No longer blocks: 1277113


8 months ago
Component: Untriaged → Developer Tools: Framework

Comment 1

8 months ago
 4:42.63 INFO: Last good revision: a98a1d78817f (2014-02-27)
 4:42.63 INFO: First bad revision: 58eca03214a6 (2014-02-28)
 4:42.63 INFO: Pushlog:

I was able to narrow down the regression window.
status-firefox50: --- → wontfix
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox53: --- → affected
Version: Trunk → 27 Branch

Comment 2

8 months ago
The regression range you got is a bit strange, because I assumed the it's regression from bug 1266134:
> https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d9413eb3db64787916c60efcb7b766cc24420d94&tochange=2268d466af6cbf62fc48006d3ff00d087c339e8b

Then OK, apparently it's not
See Also: → bug 1326697
Keywords: regressionwindow-wanted

Comment 3

8 months ago

Comment 4

7 months ago
Comment hidden (mozreview-request)

Comment 6

7 months ago
The added test covers the closeToolbox codepath which happen to be related to this bug report.
It should also cover bug 1319928.

Otherwise the patch now waits for a still loading toolbox to be fully ready before trying to close it.
Comment on attachment 8825520 [details]
Bug 1328004 - Fix race when closing the devtools while a previous instance is still loading.


Great, thanks for the fix!

::: devtools/client/framework/devtools.js:470
(Diff revision 1)
>     *         associated to the target. true, if the toolbox was successfully
>     *         closed.
>     */
> -  closeToolbox: function DT_closeToolbox(target) {
> -    let toolbox = this._toolboxes.get(target);
> +  closeToolbox: Task.async(function* (target) {
> +    let toolbox = yield this._creatingToolboxes.get(target);
>      if (toolbox == null) {

Can we just use `if (!toolbox)` here and below?  I don't think we need to specifically call out null here...?
Attachment #8825520 - Flags: review?(jryans) → review+


7 months ago
Assignee: nobody → poirot.alex
Comment hidden (mozreview-request)
Duplicate of this bug: 1330573

Comment 10

7 months ago
Pushed by apoirot@mozilla.com:
Fix race when closing the devtools while a previous instance is still loading. r=jryans
Blocks: 1327717

Comment 11

7 months ago
Last Resolved: 7 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Please request Aurora approval on this when you get a chance.
status-firefox51: affected → wontfix
Flags: needinfo?(poirot.alex)

Comment 13

7 months ago
Given that's the second regression around toolbox creation and destruction, I would like to be safe here and get some QA verification.

This is an indirect regression from bug 1266134. By the time bug 1266134 landed it wasn't that visible, some other change later make the regression more visible.
This is all about races, especially visible on slow hardware, but may be also on very fast one.

It would be good to verify that opening/closing the devtools are no longer subject to any race where we end up with broken toolbox or tool. It shouldn't be specific to the way you open/close it (like F12 versus Ctrl+I), but there may be bugs specific to which tool is opened by default.
Flags: qe-verify?


7 months ago
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(poirot.alex)
Flags: needinfo?(andrei.vaida)
Successfully managed to reproduce this bug on 52.0a1 (2016-10-28) (64-bit) (Build ID: 20161028030204) by the following Comment 0's instruction!

This issue is now Verified as Fixed on Latest Firefox Nightly 53.0a1 (2017-01-19) (64-bit)

Build ID: 20170119222621
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
OS: Linux 4.4.0-57-generic; elementary OS 0.4 (64 Bit)
QA Whiteboard: [testday-20170120]
Verified as fixed on latest Nightly 53.0a1, Buid ID 20170120030214 on Windows 7 x64bit and Mac OS X 10.10.

One note here: 
  If I hold F12 for 3 or more seconds, the devtools toolbox keeps closing and opening, and most of the time it remains closed. There are cases when the devtools toolbox remains opened, and I can close it successfully.
  Is this the intended behavior for the devtools toolbox to keep closing and opening while I hold F12 pressed?
Flags: needinfo?(andrei.vaida) → needinfo?(poirot.alex)
Alex, please fill the uplift request when you are ok!

Comment 17

7 months ago
Comment on attachment 8825520 [details]
Bug 1328004 - Fix race when closing the devtools while a previous instance is still loading.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1266134
[User impact if declined]: Possible broken toolbox on slow hardware.
[Is this code covered by automated tests?]: With this bug, yes!
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]:  Already verified.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Involved the toolbox close codepath, but this is something that is experienced a lot by most tests. I'm still surprise this regression wasn't catched by any existing test... This patch now baked for quite a bit on nightly.
[String changes made/needed]: None.
Flags: needinfo?(poirot.alex)
Attachment #8825520 - Flags: approval-mozilla-beta?
Attachment #8825520 - Flags: approval-mozilla-aurora?
Attachment #8825520 - Flags: approval-mozilla-aurora?
Based on previous comments, mark this verified on Fx 53.
status-firefox53: fixed → verified
Comment on attachment 8825520 [details]
Bug 1328004 - Fix race when closing the devtools while a previous instance is still loading.

fix race when closing devtools, beta52+
Attachment #8825520 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 20

7 months ago
status-firefox52: affected → fixed
I have reproduced this bug from STR 1 of Comment 0 with Nightly 52.0a1 (2016-10-28) on Windows 7, 64 bit.

The Bug's fix is now verified on Latest Beta 52.0b8.

Build ID   :    20170220070057
User Agent : 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
I have reproduced this bug from STR 1 of Comment 0 with Nightly 53.0a1 (2017-01-01) on Ubuntu 16.04 LTS, 64 bit!

The bug's fix is now verified on Latest Beta 52.0b8!

Build   ID :  20170220070057
User Agent :  Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0

Thanks Ikram and Maruf for verification!
Updating the tracking flags.
status-firefox52: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.