Bug 234575 – Uncaught Exception: Cannot step over because debugger is not paused

• Component: Web Inspector
• Status: RESOLVED FIXED

[Blaze Burg]

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
-----------------------

Notes:
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)

[Radar WebKit Bug Importer]

<rdar://problem/86783769>

[Patrick Angle]

Created attachment 448543 [details]
Patch v1.0

[Devin Rousso]

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

[Patrick Angle]

Created attachment 448546 [details]
Patch v1.1

[Devin Rousso]

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.

[Patrick Angle]

Created attachment 448560 [details]
Patch v1.2

[EWS]

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].