1793685 - Markup view becomes blank for cancelled navigations (eg download link)

Status: ASSIGNED

(DevTools :: Inspector, defect, P2)

Description

[hellothereleth]

Attachment: show.mp4

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:105.0) Gecko/20100101 Firefox/105.0

Steps to reproduce:

Clicked on a top level data URI and got a warning in console.

Actual results:

Inspector html content was gone, even after right clicking and hitting inspect again but style and xpath selector still show.
You have to reopen devtools to fix it.

Expected results:

Inspector html content should still be there.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'DevTools::Inspector' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Julian Descottes [:jdescottes]]

Probably similar to Bug 1347761, there is a whole class of links/navigations which seem to blank the inspector markup view

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: test_case.html

Comment 4

[Julian Descottes [:jdescottes]]

We get a will-navigate DOCUMENT_EVENT at https://searchfox.org/mozilla-central/rev/76ccfc801e6b736c844cde3fddeab7a748fc8515/devtools/client/inspector/inspector.js#351, which destroys the markup view at https://searchfox.org/mozilla-central/rev/76ccfc801e6b736c844cde3fddeab7a748fc8515/devtools/client/inspector/inspector.js#573 but since the navigation is actually cancelled, we never receive a new root node.

I wonder if we could stop monitoring will-navigate...
We already listen for ROOT_NODE and perform all the necessary cleanups when we receive a new root node.
Beyond potential test failures the only downside I see is that an "old" markup view might persist for a longer time. Otherwise maybe we can find another event but which should still be guaranteed to arrive earlier than ROOT_NODE (eg I don't think dom-complete would be a good fit?)

Comment 5

[Julian Descottes [:jdescottes]]

There are not too many failures actually, they feel test related only.
https://treeherder.mozilla.org/jobs?repo=try&revision=7602e78ac8ed0e12d63b9dac0ff8f9f516120a87

Comment 7

[Julian Descottes [:jdescottes]]

First of all, we can always cleanup the selection and the markup view when we receive a new root node. We don't necessarily have to do it earlier for the UI to be functional.

Now we might still want to do some early cleanup for the user experience: root node might arrive quite late, so displaying the markup view for the old page in the meantime can be confusing. So I tried to play a bit with other options, mostly to use pagehide, as suggested in bug 1347761.

pagehide works, but it is usually emitted quite late. In a regular webtoolbox, with all the default devtools settings, I will typically receive pagehide after receiving targetDestroyed. So it doesn't dramatically change the experience compared to what happens if we only cleanup when receiving the new root node. It also doesn't help with the few test issues in my WIP patch, because they are related to highlighter changes happening on target destroyed, and would only be fixed if we could cleanup the markup view before target destroyed.

Another option would be to only hide (not destroy) the markupview in will-navigate, and see if we can have some event/logic to show it again in case the navigation got cancelled.

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: Bug 1793685 - [devtools] Do not clean the markupview on will-navigate

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1793685 - [devtools] Destroy the markupview on pagehide

Depends on D160977

Introduce a new document event resource called pagehide.