Bug 234575 - Uncaught Exception: Cannot step over because debugger is not paused
Summary: Uncaught Exception: Cannot step over because debugger is not paused
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Patrick Angle
URL: about:blank
Keywords: InRadar
Depends on:
Reported: 2021-12-21 14:11 PST by BJ Burg
Modified: 2022-01-07 12:22 PST (History)
6 users (show)

See Also:

Patch v1.0 (2.43 KB, patch)
2022-01-06 16:00 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Patch v1.1 (3.95 KB, patch)
2022-01-06 16:46 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Patch v1.2 (3.76 KB, patch)
2022-01-06 21:33 PST, Patrick Angle
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2021-12-21 14:11:02 PST
Uncaught Exception in Web Inspector.

Steps to Reproduce:
0. Using an engineering build, launch Safari
1. Go to about:blank
2. Use keyboard shortcuts for stepping, for example Command-'

Uncaught Exceptions:
 - Cannot step over because debugger is not paused. (at DebuggerManager.js:597:44)
    stepOver @ DebuggerManager.js:597:44
    ? @ Main.js:1558:32
    _handleKeyDown @ KeyboardShortcut.js:82:42

Inspected URL:        about:blank
Loading completed:    true
Frontend User Agent:  Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko)
Comment 1 Radar WebKit Bug Importer 2021-12-21 14:11:53 PST
Comment 2 Patrick Angle 2022-01-06 16:00:18 PST
Created attachment 448543 [details]
Patch v1.0
Comment 3 Devin Rousso 2022-01-06 16:09:39 PST
Comment on attachment 448543 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=448543&action=review

> Source/WebInspectorUI/UserInterface/Base/Main.js:1556
> +        InspectorFrontendHost.beep();

I kinda feel like instead of beeping, maybe we should just disable the keyboard shortcut when not paused.  Our documentation even says "When paused" <https://webkit.org/web-inspector/keyboard-shortcuts/#sources-tab> so I don't think it's that crazy of an idea.  I'd probably look for where `WI.DebuggerManager.Event.Paused` and `WI.DebuggerManager.Event.Resumed` are dispatched and add `WI.stepOverKeyboardShortcut.disabled = !this.paused;` for each keyboard shortcut (and their alternates).

NIT: if you do wanna keep this, I'd make this into an early return
Comment 4 Patrick Angle 2022-01-06 16:46:25 PST
Created attachment 448546 [details]
Patch v1.1
Comment 5 Devin Rousso 2022-01-06 17:51:41 PST
Comment on attachment 448546 [details]
Patch v1.1

View in context: https://bugs.webkit.org/attachment.cgi?id=448546&action=review

r=me with one thing to fix :)

> Source/WebInspectorUI/UserInterface/Base/Main.js:1687
> +    WI.stepOverKeyboardShortcut.disabled = !WI.debuggerManager.paused;

NIT: Can we `let paused = WI.debuggerManager.paused;` to avoid the repeated logic?  Alternatively, you could make `WI._updateDebuggerKeyboardShortcuts` take a parameter `paused` and hardcode the value at each callsite (e.g. we know that the debugger is not paused inside `WI._debuggerDidResume` so there's kinda no reason to re-check `WI.debuggerManager.paused`, tho it would be a nice `console.assert` just to be sure).

> Source/WebInspectorUI/UserInterface/Base/Main.js:1690
> +    WI.pauseOrResumeAlternateKeyboardShortcut.disabled = !WI.debuggerManager.paused;

I dont think this should ever be disabled since it works both when and not paused.
Comment 6 Patrick Angle 2022-01-06 21:33:01 PST
Created attachment 448560 [details]
Patch v1.2
Comment 7 EWS 2022-01-07 12:22:10 PST
Committed r287776 (245836@main): <https://commits.webkit.org/245836@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448560 [details].