Summary: | Uncaught Exception: Cannot step over because debugger is not paused | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||||
Component: | Web Inspector | Assignee: | Patrick Angle <pangle> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bburg, ews-watchlist, hi, inspector-bugzilla-changes, pangle, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
URL: | about:blank | ||||||||||
Attachments: |
|
Description
BJ Burg
2021-12-21 14:11:02 PST
Created attachment 448543 [details]
Patch v1.0
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 Created attachment 448546 [details]
Patch v1.1
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. Created attachment 448560 [details]
Patch v1.2
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]. |