[remote-dbg-next] make `connect` the default page for runtimes
Categories
(DevTools :: about:debugging, enhancement, P3)
Tracking
(Not tracked)
People
(Reporter: ogasidlo, Unassigned, Mentored)
References
(Blocks 1 open bug)
Details
As we changed the order of our pages in the sidebar from this-firefox
to connect
, Daisuke suggested for https://phabricator.services.mozilla.com/D18588 to change the default page to connect
when we disconnect from the remote runtime.
After changing the route in App.js to connect
in
https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/components/App.js#99
and
and fixing 3 tests related to the default page path, another test failed and surfaced an issue that is not related to my patch.
terminal log
1:34.46 FAIL Test timed out -
1:34.61 GECKO(2745) console.warn: "Error while detaching the thread front: 'detach' request packet to 'server1.conn0.child1/context22' can't be sent as the connection is closed."
1:34.77 INFO Removing tab.
1:34.77 INFO Waiting for event: 'TabClose' on [object XULElement].
1:34.79 INFO Got event: 'TabClose' on [object XULElement].
1:34.81 GECKO(2745) console.log: MOCKED METHOD removeUSBRuntimesObserver
1:34.81 INFO Tab removed and finished closing
1:34.88 GECKO(2745) MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
1:34.88 GECKO(2745) MEMORY STAT | vsize 6837MB | residentFast 661MB | heapAllocated 300MB
console errors
Warning: Failed prop type: Invalid prop `usbRuntimes[0].extra.connectionParameters` supplied to `App`.
Warning: Failed prop type: Invalid prop `usbRuntimes[0].extra.connectionParameters` supplied to `Sidebar`.
I walked through the code and my path lead me to following issues:
Where we try to list all addons, which we do not have on the connect
page.
This is where it fails:
https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/actions/debug-targets.js#160
We do not have a runtime defined.
Daisuke, I'd love to discuss this a little bit more.
Reporter | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
Thanks Ola.
We are showing connect
as initial page when open about:debugging-new
.
Thus, I am thinking connect
page is default page already.
So, I thought, when disconnect the runtime in bug 1505128, better redirect to connect
page.
Rather, we might should redirect to /
in disconnecting case at least. Then, this of App.js handles to render default page.
What do you think?
Reporter | ||
Comment 2•5 years ago
•
|
||
Daisuke and I just had a chat.
-
We definitely want to redirect to the
connect
page (Note: theconnect
page is about to be renamed tosetup
page). -
The issue is in the
Redirect()
inApp.js
.
https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/components/App.js#99
https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/components/App.js#120
Here we need to use dispatch(Actions.selectPage(PAGE_TYPES.CONNECT))
To make this work, we also need to modify selectPage()
https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/actions/ui.js#34
As selectPage()
requires a runtimeId
, we can not provide e.g. in the case the device was disconnected (see bug 1505128).
So, every time we call selectPage()
without an existing or valid runtimeId
, we should redirect to the connect
page aka default page.
- Please note:
We might also have touch the functions indebug-targets.js
mentioned in the first comment as they also just check if we are not on thethis-firefox
page. This worked well, when the default page wasthis-firefox
. But since we changed this, we need to also check for aruntime
at all.
Related console.error message to that is:
[ACTION FAILED] REQUEST_EXTENSIONS_FAILURE: Connection closed, pending request to root, type listAddons failed
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Updated•2 years ago
|
Description
•