892560 - [inspector] reloading the page breaks the inspector

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Julien Wajsberg [:julienw]]

STR:
* open a webpage
* open the dev tools
* open the inspector tab
* reload the page

=> the inspector tab is irresponsive
=> trying to close the dev tools doesn't work

The only way to recover is to restart the browser.

Comment 1

[Paul Rouget [:paul]]

Exceptions:

--
[13:48:05.664] TypeError: this._walker is null @ resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/inspector/selection.js:122
--
[13:51:35.042] TypeError: this.undo is undefined @ resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/markupview/markup-view.js:646

Comment 4

[Loic]

good=2013-07-04
bad=2013-07-05
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dcbbfcdf7bb4&tochange=17fe59f6c54a

Browser console after clicking on the close button:
[00:47:41.222] TypeError: this.undo is undefined @ resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/markupview/markup-view.js:646

Suspected bug:
Michael Ratcliffe — Bug 852996 - MarkupView leaks the whole Inspector after being closed r=dcamp

Comment 5

[Paul Rouget [:paul]]

Attachment: Patch v1

This patch fixes 3 things:
- if a panel fails to close, it should not stop the toolbox to get destroyed;
- we get a "dead object" exception while resetting the markup view;
- the highlighter should use isConnected() first as it checks for the dead object exception;

Comment 6

[Dave Camp (:dcamp)]

Comment on attachment 774597 [details] [diff] [review]
Patch v1

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

Why wasn't this caught by our reloading test?  Can we add a test that does catch it?

::: browser/devtools/framework/toolbox.js
@@ +755,5 @@
>      let outstanding = [];
>  
> +    try {
> +      for (let [id, panel] of this._toolPanels) {
> +        outstanding.push(panel.destroy());

Shouldn't you put the try around each panel.destroy() call, so that all the tools at least have a chance to fail?

Comment 7

[Paul Rouget [:paul]]

Attachment: Patch v1.1

Addressed comments (includes a test that fails before fix).

Comment 8

[Paul Rouget [:paul]]

https://tbpl.mozilla.org/?tree=Try&rev=1e0e3f51ef07

Comment 9

[Paul Rouget [:paul]]

https://tbpl.mozilla.org/?tree=Try&rev=6afb3dc90dcf

Comment 11

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/7d182fb1c981

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/7d182fb1c981

Comment 14

[Ioana (away)]

Verified as fixed on Firefox 25 beta 1, on Ubuntu 13.04 46bit:
Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20100101 Firefox/25.0 (20130917123208)

Comment 15

[billmalcolm3]

Hi, 
I think this bug, or a similar type is affecting Firefox Developer Edition 61.0b3 (64-bit).

When I open up the Inspector, it shows the contents of a div just fine, but if I reload the page with the inspector open, whatever div I had selected now shows up as having no contents in the Inspector. 

Is it possible to reopen this bug directed at FFDE or should I create a new one? 

Thanks, and have a great day!

Comment 16

[billmalcolm3]

Seems to only happen if I am inspecting something on localhost and changes are made. Not terribly consistent either, which is a huge bummer. Windows 10 Pro

Comment 17

[Patrick Brosset <:pbro>]

Hi billmalcolm3, this is probably a different bug, since this one here was closed 5 years ago.
If you have a good set of steps someone could follow to reproduce the issue, I suggest opening a new bug at https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox&component=Developer%20Tools%3A%20Inspector

By your description of the issue, I think it might be the same problem as bug 1460223 which I have a fix for.