[webkit-reviews] review granted: [Bug 200275] Web Inspector: Debugger: allow breakpoint actions to be evaluated as a user gesture : [Attachment 417326] Patch

[bugzilla-daemon at webkit.org]

BJ Burg <bburg at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 200275: Web Inspector: Debugger: allow breakpoint actions to be evaluated
as a user gesture
https://bugs.webkit.org/show_bug.cgi?id=200275

Attachment 417326: Patch

https://bugs.webkit.org/attachment.cgi?id=417326&action=review




--- Comment #4 from BJ Burg <bburg at apple.com> ---
Comment on attachment 417326
  --> https://bugs.webkit.org/attachment.cgi?id=417326
Patch

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

r=me, nice work!

> LayoutTests/inspector/debugger/breakpoint-action-emulateUserGesture.html:14
> +	   TestPage.addResult(window.internals.isProcessingUserGesture() ? "In
User Gesture" : "Not in User Gesture");

What's the difference between .isProcessingUserGresture() here and
.userIsInteracting() above?

> Source/JavaScriptCore/debugger/Debugger.cpp:618
> +    JSObject* scopeExtensionObject = m_client ?
m_client->debuggerScopeExtensionObject(*this, globalObject, debuggerCallFrame)
: nullptr;

Nice.

> Source/WebCore/inspector/agents/page/PageDebuggerAgent.cpp:126
> +	  
m_breakpointActionUserGestureEmulationScopeStack.append(makeUniqueRef<UserGestu
reEmulationScope>(m_inspectedPage, true));

What a mouthful. I like the stack idea though.

> Source/WebInspectorUI/UserInterface/Views/BreakpointActionView.js:139
> +	      
emulateUserGestureLabel.appendChild(document.createTextNode(WI.UIString("Emulat
e User Gesture")));

Please add a location to the UIString (@ Evaluate JavaScript breakpoint action
menu in Sources tab)

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:84
> +	       this._emulateUserGestureNavigationItem = new
WI.CheckboxNavigationItem("emulate-in-user-gesture", WI.UIString("Emulate User
Gesture"), WI.settings.emulateInUserGesture.value);

Please add a location to the UIString (@ Option in Console Tab)