Open Bug 1562165 Opened 5 years ago Updated 2 years ago

Adding a watch expression show the WhyPaused element and hides it in a few ms

Categories

(DevTools :: Debugger, defect, P3)

defect

Tracking

(Not tracked)

People

(Reporter: nchevobbe, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached video Jun-28-2019 11-08-08.mp4 (deleted) —

Steps to reproduce

  1. Open the debugger in any page
  2. Make sure you're not paused
  3. In the watch expression panel, evaluate 1 + 1

Actual results

The yellow, WhyPaused element, briefly appears (see attached video)

I can reproduce that bug, thanks for the report Nicolas!
Honza

Priority: -- → P3

If I do the following:

diff --git a/devtools/client/debugger/src/components/SecondaryPanes/WhyPaused.js b/devtools/client/debugger/src/components/SecondaryPanes/WhyPaused.js
--- a/devtools/client/debugger/src/components/SecondaryPanes/WhyPaused.js
+++ b/devtools/client/debugger/src/components/SecondaryPanes/WhyPaused.js
@@ -79,7 +79,7 @@ class WhyPaused extends PureComponent<Pr
     const reason = getPauseReason(why);
 
     if (!reason || endPanelCollapsed) {
-      return <div className={this.state.hideWhyPaused} />;
+      return null;
     }
 
     return (

the issue goes away.
I'm not sure why we are rendering an empty div when we don't have a reason to be paused?

(also, I'd set this higher than a P3, since it makes the app looks unpolished)

David, what do you think about the fix above?

Nicolas, should I assign it to you?

Honza

Flags: needinfo?(dwalsh)

I'm not really sure, maybe there's a reason we're rendering this empty div?
Maybe it's done to not shift the layout around when stepping in/over (which might explain the delay in componentDidUpdate)

That's right - it's to make it so that the panels in the right sidebar don't bounce up and down while stepping. Original PR: https://github.com/firefox-devtools/debugger/pull/8163.

As Jaril said, we render the “empty” yellow div to keep the height and display of the element in the case that a “pause-resume-pause” happens quickly, which is how stepping works. If we don’t have this delay, there’s a very obvious stutter every single time a users steps.

I tried debouncing our updatePausedState function but that breaks things. Perhaps there’s something else we can do.

Flags: needinfo?(dwalsh)

This is using CSS animation with a delay to still handle cases
where we pause right after resuming (e.g. in a loop).
We only need to add a state property for the initial render so we
don't briefly render the component when starting the debugger (if
it's not paused).

Not sure if this is the way to go since it might be less easy to test.

David, I tried something on https://phabricator.services.mozilla.com/D36856, based on CSS animation + delay.
Could you have a look and tell me if it looks okay? If you think so, I'll see if there are issues with tests.
Thanks!

Flags: needinfo?(dwalsh)
Flags: needinfo?(dwalsh)
Attachment #9075858 - Attachment is obsolete: true
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: