1967931 - Devtools highlighters are not visible when debugger is paused (e.g. Paused overlay is not displayed anymore)

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Nicolas Chevobbe [:nchevobbe]]

Steps to reproduce

1. Go to https://nchevobbe.github.io/demo/console-test-app.html
2. Open the debugger
3. In the page, click on the "Pause in debugger" button

Expected results

The debugger pauses, and the paused overlay is displayed in the content page

Actual results

The debugger pauses, but the paused overlay isn't displayed in the content page

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

mozregression points to Bug 1959644

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Note that the devtools/client/debugger/test/mochitest/browser_dbg-paused* tests are running fine.
It seems like the overlay elements are properly rendered and should be visible, but the overlay isn't visible.
Maybe the fact that the page is paused is the issue here, once I'm in that state, the other highlighters aren't working either

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Right, the main issue is that the page being paused means that we're not supposed to run stuff like requestAnimationFrame, layout updates / css transitions / etc, nor painting. My patches in bug 1959644 are only part of the thing there, bug 1958942 is the bigger change, really.

Would it be too hard to make the pause overlay work on the front-end (as an overlay on the <browser> for example) rather than as part of the page?

Comment 4

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

Set release status flags based on info from the regressing bug 1959644

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

Right, the main issue is that the page being paused means that we're not supposed to run stuff like requestAnimationFrame, layout updates / css transitions / etc, nor painting. My patches in bug 1959644 are only part of the thing there, bug 1958942 is the bigger change, really.

Oh, so that breaks even more things then. For example modifying some rules declaration in the rules view, removing an element from the markup view, while the debugger is paused; now those are not visible until you resume. The ability to do some changes on the page when the debugger is paused is quite valuable IMO (and something that other browser devtools allow too), I think this makes this issue a higher priority, I'll discuss it with the team.

Would it be too hard to make the pause overlay work on the front-end (as an overlay on the <browser> for example) rather than as part of the page?

I don't really know from the top of my head. Not that it doesn't impact only the paused overlay, but all the highlighters (so for example, using the node pickers when the debugger is paused).
FWIW I was also envisioning taking advantage of CSS anchor positioning once it's there to be able to position element-related highlighters more easily (instead of asking for their bounding box, and applying some math to position the highlighters elements). This wouldn't be an option if the highlighters don't live in the page document.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

I guess another option would be to still paint and run layout and so on for this case, something like:

diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp
index 31f0581a4f82..6db48ed32c67 100644
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -7413,7 +7413,8 @@ bool Document::IsRenderingSuppressed() const {
   }
   // The user agent believes that updating the rendering of doc's node navigable
   // would have no visible effect.
-  if (!IsEventHandlingEnabled() && !IsBeingUsedAsImage() && !mDisplayDocument) {
+  if (!IsEventHandlingEnabled() && !IsBeingUsedAsImage() && !mDisplayDocument &&
+      !DevToolsAnonymousAndShadowEventsEnabled()) {
     return true;
   }
   if (!mPresShell || !mPresShell->DidInitialize()) {

That would prevent the rendering from being suppressed while the debugger is paused, maybe it's fine? It does mean that the page's requestAnimationFrame callbacks and such could run unexpectedly, maybe? But then again before my changes a lot of things could run like ResizeObserver / IntersectionObserver / etc which now don't...

Comment 7

[Julian Descottes [:jdescottes]]

Just to confirm: it's important that regular DOM highlighters still work when paused, as pausing with the debugger is sometimes the only way developers can debug a layout (think a popup which auto hides for instance). It's usually been our recommendation to pause with the debugger in those cases.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1967931 - Avoid blocking rendering for devtools pauses. r=smaug

Not a huge fan but this would do...

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

ok, so "pausing" isn't supposed to pause any layout stuff? I wonder if all the animations are still also running

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Yeah to me it feels more "natural" that pausing would actually pause CSS animations / transitions / etc... But given comment 7 that doesn't seem what folks want / expect?

Comment 11

[Holger Benl [:hbenl]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

Yeah to me it feels more "natural" that pausing would actually pause CSS animations / transitions / etc... But given comment 7 that doesn't seem what folks want / expect?

Pausing CSS animations etc. would feel natural but on the other hand not being able to test CSS changes while being paused would be a major limitation (and also very confusing unless we completely disable editing CSS in devtools while paused).

Comment 12

[Alexandre Poirot [:ochameau]]

Pausing animations has been a recurrent old topic, which was never tackled by lack of time and expertise.
See bug 1200425, this would most likely not be bound to debugger pause by default, and so would require some thoughts about UI.
Also see bug 1513727 about run-to-completion issues related to animations, which is yet another issue.

Comment 13

[Emilio Cobos Álvarez [:emilio]]

There are a couple tests failing with this change. Some of them about e.g. scroll events not firing, see devtools/client/debugger/test/mochitest/browser_dbg-scroll-run-to-completion.js. Recently I've unified all our rendering loop. We could selectively fire some but not other events, but that'd be rather weird, and eventually we'd like to move to HTML spec behavior which does this check upfront just once for all documents... Any strong opinions or suggestions?

Comment 14

[Julian Descottes [:jdescottes]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

Yeah to me it feels more "natural" that pausing would actually pause CSS animations / transitions / etc... But given comment 7 that doesn't seem what folks want / expect?

I only meant that people should be able to use the highlighters when the debugger is paused.
If you are using this to debug the layout of a tooltip/popup which gets shown / hidden with JS.

Then as :ochameau said, having the ability to fully "freeze" the page is a long standing request for devtools.
I just meant that devtools (incl. highlighters) should remain functional when the debugger is paused.

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

Yeah to me it feels more "natural" that pausing would actually pause CSS animations / transitions / etc... But given comment 7 that doesn't seem what folks want / expect?

Testing with data:text/html,<meta charset=utf8><style>body { animation: 2s a infinite linear; } @keyframes a { from { background: blue; } to { background: red; }%20 }</style><h1>Hello</h1><script>document.addEventListener("click", () => {debugger;})</script> , clicking on the page to pause in the Debugger, Chrome and Safari don't pause the animation. Is this a discussion that should be brought up at the spec level somehow?

Comment 16

[Nicolas Chevobbe [:nchevobbe]]

Adding here what I said in https://phabricator.services.mozilla.com/D250772#8668692 :

While paused we used to be able to modify the page in various ways:

• from the markup view (adding nodes, editing attribute values, …)
• from the rules view (adding a new rule, updating a declaration, simulating dark mode, …)
• from the layout panel (updating the box dimensions)
• from the fonts panel (changing the size/weight of a font)
• from the console (executing any expression the user provides)
• from the style editor (editing/adding stylesheets)

and that's something that also work in Chrome and Safari, I don't think we should differ from them here.
Now how we achieve this might be another story, we could set explicit flags/call specific method when handling those various scenario so we would trigger a rendering, but that's also something I fear might be brittle / easy to forget when adding new ways to update the elements on the page.

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Ok, I fixed it up to be specific about the DevTools pause, rather than highlighters, which I think addresses Alexandre's concern. It should allow all the things in comment 16.

This patch regresses browser_dbg-scroll-run-to-completion.js, but I think that is acceptable. The scroll events have a very specific timing per spec. It's a bit weird to suppress scroll events but none of the other steps in the rendering loop (media query listener events, resize vents, rAF callbacks, IntersectionObserver callbacks, ResizeObserver callbacks).

I think we should take a better look at https://html.spec.whatwg.org/#update-the-rendering, and if there is a reasoning to delay some but not all phases, we should document it. It's not unimplementable, but it's rather weird...

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57013ce97ef3
Avoid blocking rendering for devtools pauses. r=smaug,devtools-reviewers,nchevobbe

Comment 19

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/57013ce97ef3