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

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[Maria Berlinger [:mberlinger], Services QA]

Attachment: 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.

Comment 1

[Marion Daly [:mdaly]]

Hi Patrick, could you help us here?

Comment 2

[Patrick Brosset <:pbro>]

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

Comment 3

[Patrick Brosset <:pbro>]

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.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

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?

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Ah, sorry, this slipped off my radar, working on it now.

Honza

Comment 6

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: Bug 1477252 - Check whether editor is destroyed before using it; review=nchevobbe

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99df2932f5d5
Check whether editor is destroyed before using it; review=nchevobbe r=nchevobbe

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/99df2932f5d5

Comment 9

[Peter Magyari (Desktop QA)]

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).

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance. It grafts cleanly as-landed.

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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:

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 9018603 [details]
Bug 1477252 - Check whether editor is destroyed before using it; review=nchevobbe

netmonitor fix for 64.0b7

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/releases/mozilla-beta/rev/013f4e5f93fd

Comment 14

[Maria Berlinger [:mberlinger], Services QA]

This issue is verified fixed using Firefox 64.0b7 on the following OSes: Windows 10 x64, mac 10.13.6, Ubuntu 16.04 x64.