Closed Bug 1477252 Opened Last year Closed Last year

After scrolling a few seconds in the request list with arrow keys the network's panel becomes blank

Categories

(DevTools :: Netmonitor, defect, P1, major)

60 Branch
defect

Tracking

(firefox-esr60 wontfix, firefox61 wontfix, firefox62 wontfix, firefox63 wontfix, firefox64 verified, firefox65 verified)

VERIFIED FIXED
Firefox 65
Tracking Status
firefox-esr60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- verified
firefox65 --- verified

People

(Reporter: maria_berlinger, Assigned: Honza)

References

Details

(Keywords: regression)

Attachments

(2 files)

Attached image bug capture.gif
[Affected versions]:
Firefox 63.0a1 (BuildId:20180718100918).
Firefox 62.0b9 (BuildId:20180713213322).
Firefox 61.0.1 (BuildId:20180704003137).
Firefox 60.1.0 esr (BuildId:20180621121604).

[Affected platforms]:
Windows 10 64bit.
macOS 10.13.4
Ubuntu 16.04 64bit.

[Steps to reproduce]:
1. Launch Firefox.
2. Go to any website (e.g. https://www.youtube.com/).
3. Right click on the page and choose "Inspect Element".
4. From the dev tools panel go to "Network".
5. Click a random request.
6. From "Details pane" choose "Response" tab.
7. Scroll up/down on requests with arrow keys for a while.

[Expected result]:
The requests are properly scrolled using the up/down arrow keys.  

[Actual result]:
After scrolling a few seconds in the request list the network's panel becomes blank.

[Regression range]:
This seems to be a regression:

Last good revision: 0c39c734b41929d2de8ed1f090b51bca95fefb9e
First bad revision: 3fb1f0afdc6decd6d53876304c0806cc6b7d39d0

Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0c39c734b41929d2de8ed1f090b51bca95fefb9e&tochange=3fb1f0afdc6decd6d53876304c0806cc6b7d39d0

[Additional information]:
For further information regarding this issue please observe the attached screencast.
Hi Patrick, could you help us here?
Flags: needinfo?(pbrosset)
I reproduced it on Windows 10/Nightly 63.
This is a P1 for sure since the entire panel disappears, and this is likely to happen to users primarily using the keyboard.

My hunch is that as you navigate through the list with the keyboard, the response sidebar tries to refresh as you go. But navigation is so fast that the content of that sidebar sometimes does not have time to load (it does it async). So it may sometimes be that the refresh starts, but by the time it ends, it's too late, the user has already selected another item from the list.

I think that our code here doesn't expect that to happen, and just fails.

It's a bit sad that the entire panel becomes blank. It would be better if the error occurred but didn't crash the entire panel.
Having said that, we do need to fix it anyway.

Here's the JS error in the browser console:

TypeError: this.config is null
editor.js:1231:7

TypeError: "cm is undefined"
setOptionresource://devtools/client/sourceeditor/editor.js:1230:7setModeresource://devtools/client/sourceeditor/editor.js:498:5componentDidUpdateresource://devtools/client/netmonitor/src/components/SourceEditor.js:61:7commitLifeCyclesresource://devtools/client/shared/vendor/react-dom.js:11399:13commitAllLifeCyclesresource://devtools/client/shared/vendor/react-dom.js:12348:7commitRootresource://devtools/client/shared/vendor/react-dom.js:12492:9completeRootresource://devtools/client/shared/vendor/react-dom.js:13417:34performWorkOnRootresource://devtools/client/shared/vendor/react-dom.js:13362:9performWorkresource://devtools/client/shared/vendor/react-dom.js:13281:7performSyncWorkresource://devtools/client/shared/vendor/react-dom.js:13253:31resource://devtools/client/shared/vendor/react-dom.js:13518:7interactiveUpdatesresource://devtools/client/shared/vendor/react-dom.js:2041:10dispatchInteractiveEventresource://devtools/client/shared/vendor/react-dom.js:4318:3addEventBubbleListenerresource://devtools/client/shared/vendor/react-dom.js:3745:3trapBubbledEventresource://devtools/client/shared/vendor/react-dom.js:4292:3listenToresource://devtools/client/shared/vendor/react-dom.js:4488:13ensureListeningToresource://devtools/client/shared/vendor/react-dom.js:6087:31resource://devtools/client/shared/vendor/react-dom.js:6244:7finalizeInitialChildrenresource://devtools/client/shared/vendor/react-dom.js:6771:3completeWorkresource://devtools/client/shared/vendor/react-dom.js:11178:17completeUnitOfWorkresource://devtools/client/shared/vendor/react-dom.js:12591:18performUnitOfWorkresource://devtools/client/shared/vendor/react-dom.js:12718:12workLoopresource://devtools/client/shared/vendor/react-dom.js:12730:24renderRootresource://devtools/client/shared/vendor/react-dom.js:12770:7performWorkOnRootresource://devtools/client/shared/vendor/react-dom.js:13359:22performWorkresource://devtools/client/shared/vendor/react-dom.js:13281:7performSyncWorkresource://devtools/client/shared/vendor/react-dom.js:13253:3requestWorkresource://devtools/client/shared/vendor/react-dom.js:13153:51resource://devtools/client/shared/vendor/react-dom.js:13022:11scheduleRootUpdateresource://devtools/client/shared/vendor/react-dom.js:13566:3updateContainerAtExpirationTimeresource://devtools/client/shared/vendor/react-dom.js:13581:10updateContainerresource://devtools/client/shared/vendor/react-dom.js:13608:10renderresource://devtools/client/shared/vendor/react-dom.js:13853:3legacyRenderSubtreeIntoContainerresource://devtools/client/shared/vendor/react-dom.js:13965:9unbatchedUpdatesresource://devtools/client/shared/vendor/react-dom.js:13478:10legacyRenderSubtreeIntoContainerresource://devtools/client/shared/vendor/react-dom.js:13961:5renderresource://devtools/client/shared/vendor/react-dom.js:14012:12bootstrapresource://devtools/client/netmonitor/src/app.js:64:5openresource://devtools/client/netmonitor/panel.js:24:11onLoadresource://devtools/client/framework/toolbox.js:1710:21
Severity: normal → major
Flags: needinfo?(pbrosset)
Priority: -- → P1
Looks like it's a case where the codemirror editor (which we use to display the request's response) is being destroyed before we try to access some of its internals.

In devtools\client\sourceeditor\editor.js in many places we access the codemirror instance by doing:
const cm = editors.get(this);
and this can return undefined (presumably if the editor was already destroyed, or not created yet).
So in theory, everywhere we do this, we should guard against it being undefined, and bail out.
Too late for a fix for 63 but we can still take a patch for 65/64. 
Honza, can you help find an owner for this bug?
Flags: needinfo?(odvarko)
Ah, sorry, this slipped off my radar, working on it now.

Honza
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Flags: needinfo?(odvarko)
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/99df2932f5d5
Check whether editor is destroyed before using it; review=nchevobbe r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/99df2932f5d5
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Verified as fixed on Firefox Nightly 65.0a1 (2018-11-01)(64-bit) on Windows 10(x64), Mac OS X 10.13 and on Ubuntu 16.04(x64).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Please nominate this for Beta approval when you get a chance. It grafts cleanly as-landed.
Flags: needinfo?(odvarko)
Comment on attachment 9018603 [details]
Bug 1477252 - Check whether editor is destroyed before using it; review=nchevobbe

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: The Network panel (DevTools) breaks when the user navigates in it using the keyboard.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The patch is rather smaller, tested and affects only Web developers.

String changes made/needed:
Flags: needinfo?(odvarko)
Attachment #9018603 - Flags: approval-mozilla-beta?
Comment on attachment 9018603 [details]
Bug 1477252 - Check whether editor is destroyed before using it; review=nchevobbe

netmonitor fix for 64.0b7
Attachment #9018603 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
This issue is verified fixed using Firefox 64.0b7 on the following OSes: Windows 10 x64, mac 10.13.6, Ubuntu 16.04 x64.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.