Web Inspector: we should show artificial context menus on mousedown instead of click
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2019 08:00:56 +0000 (08:00 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Mar 2019 08:00:56 +0000 (08:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195494

Reviewed by Joseph Pecoraro.

* UserInterface/Views/ContextMenu.js:
(WI.ContextMenu):
(WI.ContextMenu.prototype.show):
(WI.ContextMenu.prototype.addBeforeShowCallback): Added.
(WI.ContextMenu.prototype.handleEvent):
Provide a way to register a callback that will be called right as the "contextmenu" event is
handled, but before the context menu is actually shown. Since "mousedown" events are also
fired when/before a "contextmenu" event is fired, each of the below callers has to maintain
some state indicating "we are about to show a context menu, so ignore all "mousedown" events
until that time". Without this, the below callers wouldn't be able to tell when the context
menu is finally shown.

* UserInterface/Base/SearchUtilities.js:
(WI.SearchUtilities.createSettingsButton):
* UserInterface/Views/CanvasContentView.js:
(WI.CanvasContentView):
(WI.CanvasContentView.prototype.initialLayout):
(WI.CanvasContentView.prototype._handleCanvasElementButtonMouseDown): Added.
(WI.CanvasContentView.prototype._handleViewShaderButtonMouseDown): Added.
(WI.CanvasContentView.prototype._handleViewRecordingButtonMouseDown): Added.
(WI.CanvasContentView.prototype._canvasElementButtonClicked): Deleted.
(WI.CanvasContentView.prototype._handleViewShaderButtonClicked): Deleted.
(WI.CanvasContentView.prototype._handleViewRecordingButtonClicked): Deleted.
* UserInterface/Views/DebuggerSidebarPanel.js:
(WI.DebuggerSidebarPanel):
(WI.DebuggerSidebarPanel.prototype._handleCreateBreakpointMouseDown): Added.
(WI.DebuggerSidebarPanel.prototype._handleCreateBreakpointClicked): Deleted.
* UserInterface/Views/SourcesNavigationSidebarPanel.js:
(WI.SourcesNavigationSidebarPanel):
(WI.SourcesNavigationSidebarPanel.prototype._handleCreateBreakpointMouseDown): Added.
(WI.SourcesNavigationSidebarPanel.prototype._handleCreateBreakpointClicked): Deleted.
* UserInterface/Views/TabBar.js:
(WI.TabBar.prototype._handleMouseDown):
* UserInterface/Views/LegacyTabBar.js:
(WI.LegacyTabBar.prototype._handleMouseDown):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242937 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Base/SearchUtilities.js
Source/WebInspectorUI/UserInterface/Views/CanvasContentView.js
Source/WebInspectorUI/UserInterface/Views/ContextMenu.js
Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
Source/WebInspectorUI/UserInterface/Views/LegacyTabBar.js
Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js
Source/WebInspectorUI/UserInterface/Views/TabBar.js

index 9e4afa5..6e9da2b 100644 (file)
@@ -1,3 +1,46 @@
+2019-03-14  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: we should show artificial context menus on mousedown instead of click
+        https://bugs.webkit.org/show_bug.cgi?id=195494
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Views/ContextMenu.js:
+        (WI.ContextMenu):
+        (WI.ContextMenu.prototype.show):
+        (WI.ContextMenu.prototype.addBeforeShowCallback): Added.
+        (WI.ContextMenu.prototype.handleEvent):
+        Provide a way to register a callback that will be called right as the "contextmenu" event is
+        handled, but before the context menu is actually shown. Since "mousedown" events are also
+        fired when/before a "contextmenu" event is fired, each of the below callers has to maintain
+        some state indicating "we are about to show a context menu, so ignore all "mousedown" events
+        until that time". Without this, the below callers wouldn't be able to tell when the context
+        menu is finally shown.
+
+        * UserInterface/Base/SearchUtilities.js:
+        (WI.SearchUtilities.createSettingsButton):
+        * UserInterface/Views/CanvasContentView.js:
+        (WI.CanvasContentView):
+        (WI.CanvasContentView.prototype.initialLayout):
+        (WI.CanvasContentView.prototype._handleCanvasElementButtonMouseDown): Added.
+        (WI.CanvasContentView.prototype._handleViewShaderButtonMouseDown): Added.
+        (WI.CanvasContentView.prototype._handleViewRecordingButtonMouseDown): Added.
+        (WI.CanvasContentView.prototype._canvasElementButtonClicked): Deleted.
+        (WI.CanvasContentView.prototype._handleViewShaderButtonClicked): Deleted.
+        (WI.CanvasContentView.prototype._handleViewRecordingButtonClicked): Deleted.
+        * UserInterface/Views/DebuggerSidebarPanel.js:
+        (WI.DebuggerSidebarPanel):
+        (WI.DebuggerSidebarPanel.prototype._handleCreateBreakpointMouseDown): Added.
+        (WI.DebuggerSidebarPanel.prototype._handleCreateBreakpointClicked): Deleted.
+        * UserInterface/Views/SourcesNavigationSidebarPanel.js:
+        (WI.SourcesNavigationSidebarPanel):
+        (WI.SourcesNavigationSidebarPanel.prototype._handleCreateBreakpointMouseDown): Added.
+        (WI.SourcesNavigationSidebarPanel.prototype._handleCreateBreakpointClicked): Deleted.
+        * UserInterface/Views/TabBar.js:
+        (WI.TabBar.prototype._handleMouseDown):
+        * UserInterface/Views/LegacyTabBar.js:
+        (WI.LegacyTabBar.prototype._handleMouseDown):
+
 2019-03-13  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Debugger: pausing in an inline script on a page with a URL query creates an Extra Script
index c792e6c..5a58656 100644 (file)
@@ -72,11 +72,21 @@ WI.SearchUtilities = class SearchUtilities {
     {
         console.assert(!isEmptyObject(settings));
 
+        let ignoreMouseDown = false;
+
         let button = document.createElement("button");
-        button.addEventListener("click", (event) => {
+        button.addEventListener("mousedown", (event) => {
             event.stop();
 
+            if (ignoreMouseDown)
+                return;
+
+            ignoreMouseDown = true;
+
             let contextMenu = WI.ContextMenu.createFromEvent(event);
+            contextMenu.addBeforeShowCallback(() => {
+                ignoreMouseDown = false;
+            });
 
             if (settings.caseSensitive) {
                 contextMenu.appendCheckboxItem(WI.UIString("Case Sensitive", "Case Sensitive @ Context Menu", "Context menu label for whether searches should be case sensitive."), () => {
index 062efcc..7caea02 100644 (file)
@@ -113,7 +113,7 @@ WI.CanvasContentView = class CanvasContentView extends WI.ContentView
 
             let canvasElementButtonNavigationItem = new WI.ButtonNavigationItem("canvas-element", WI.UIString("Canvas Element"), "Images/Markup.svg", 16, 16);
             canvasElementButtonNavigationItem.visibilityPriority = WI.NavigationItem.VisibilityPriority.Low;
-            canvasElementButtonNavigationItem.addEventListener(WI.ButtonNavigationItem.Event.Clicked, this._canvasElementButtonClicked, this);
+            canvasElementButtonNavigationItem.element.addEventListener("mousedown", this._handleCanvasElementButtonMouseDown.bind(this));
             navigationBar.addNavigationItem(canvasElementButtonNavigationItem);
 
             navigationBar.addNavigationItem(this._refreshButtonNavigationItem);
@@ -134,12 +134,12 @@ WI.CanvasContentView = class CanvasContentView extends WI.ContentView
             this._viewShaderButton = document.createElement("img");
             this._viewShaderButton.classList.add("view-shader");
             this._viewShaderButton.title = WI.UIString("View Shader");
-            this._viewShaderButton.addEventListener("click", this._handleViewShaderButtonClicked.bind(this));
+            this._viewShaderButton.addEventListener("mousedown", this._handleViewShaderButtonMouseDown.bind(this));
 
             this._viewRecordingButton = document.createElement("img");
             this._viewRecordingButton.classList.add("view-recording");
             this._viewRecordingButton.title = WI.UIString("View Recording");
-            this._viewRecordingButton.addEventListener("click", this._handleViewRecordingButtonClicked.bind(this));
+            this._viewRecordingButton.addEventListener("mousedown", this._handleViewRecordingButtonMouseDown.bind(this));
 
             this._updateViewRelatedItems();
 
@@ -321,9 +321,17 @@ WI.CanvasContentView = class CanvasContentView extends WI.ContentView
         });
     }
 
-    _canvasElementButtonClicked(event)
+    _handleCanvasElementButtonMouseDown(event)
     {
-        let contextMenu = WI.ContextMenu.createFromEvent(event.data.nativeEvent);
+        if (this._ignoreCanvasElementButtonMouseDown)
+            return;
+
+        this._ignoreCanvasElementButtonMouseDown = true;
+
+        let contextMenu = WI.ContextMenu.createFromEvent(event);
+        contextMenu.addBeforeShowCallback(() => {
+            this._ignoreCanvasElementButtonMouseDown = false;
+        });
 
         if (this._canvasNode)
             WI.appendContextMenuItemsForDOMNode(contextMenu, this._canvasNode);
@@ -428,8 +436,11 @@ WI.CanvasContentView = class CanvasContentView extends WI.ContentView
             this._viewRelatedItemsContainer.appendChild(this._viewRecordingButton);
     }
 
-    _handleViewShaderButtonClicked(event)
+    _handleViewShaderButtonMouseDown(event)
     {
+        if (this._ignoreViewShaderButtonMouseDown)
+            return;
+
         let shaderPrograms = this.representedObject.shaderProgramCollection;
         console.assert(shaderPrograms.size);
         if (!shaderPrograms.size)
@@ -440,10 +451,15 @@ WI.CanvasContentView = class CanvasContentView extends WI.ContentView
             return;
         }
 
+        this._ignoreViewShaderButtonMouseDown = true;
+
         let contextMenu = WI.ContextMenu.createFromEvent(event);
+        contextMenu.addBeforeShowCallback(() => {
+            this._ignoreViewShaderButtonMouseDown = false;
+        });
 
         for (let shaderProgram of shaderPrograms) {
-            contextMenu.appendItem(shaderProgram.displayName, (event) => {
+            contextMenu.appendItem(shaderProgram.displayName, () => {
                 WI.showRepresentedObject(shaderProgram);
             });
         }
@@ -451,8 +467,11 @@ WI.CanvasContentView = class CanvasContentView extends WI.ContentView
         contextMenu.show();
     }
 
-    _handleViewRecordingButtonClicked(event)
+    _handleViewRecordingButtonMouseDown(event)
     {
+        if (this._ignoreViewRecordingButtonMouseDown)
+            return;
+
         let recordings = this.representedObject.recordingCollection;
         console.assert(recordings.size);
         if (!recordings.size)
@@ -463,10 +482,15 @@ WI.CanvasContentView = class CanvasContentView extends WI.ContentView
             return;
         }
 
+        this._ignoreViewRecordingButtonMouseDown = true;
+
         let contextMenu = WI.ContextMenu.createFromEvent(event);
+        contextMenu.addBeforeShowCallback(() => {
+            this._ignoreViewRecordingButtonMouseDown = false;
+        });
 
         for (let recording of recordings) {
-            contextMenu.appendItem(recording.displayName, (event) => {
+            contextMenu.appendItem(recording.displayName, () => {
                 WI.showRepresentedObject(recording);
             });
         }
index 47a6b68..95f7bee 100644 (file)
@@ -157,6 +157,8 @@ WI.ContextMenu = class ContextMenu extends WI.ContextSubMenuItem
         this._event = event;
         this._handlers = {};
         this._id = 0;
+
+        this._beforeShowCallbacks = [];
     }
 
     // Static
@@ -197,6 +199,8 @@ WI.ContextMenu = class ContextMenu extends WI.ContextSubMenuItem
             WI.ContextMenu._lastContextMenu = this;
 
             if (this._event.type !== "contextmenu" && typeof InspectorFrontendHost.dispatchEventAsContextMenuEvent === "function") {
+                console.assert(event.type !== "mousedown" || this._beforeShowCallbacks.length > 0, "Calling show() in a mousedown handler should have a before show callback to enable quick selection.");
+
                 this._menuObject = menuObject;
                 this._event.target.addEventListener("contextmenu", this, true);
                 InspectorFrontendHost.dispatchEventAsContextMenuEvent(this._event);
@@ -208,10 +212,20 @@ WI.ContextMenu = class ContextMenu extends WI.ContextSubMenuItem
             this._event.stopImmediatePropagation();
     }
 
+    addBeforeShowCallback(callback)
+    {
+        this._beforeShowCallbacks.push(callback);
+    }
+
     // Protected
 
     handleEvent(event)
     {
+        console.assert(event.type === "contextmenu");
+
+        for (let callback of this._beforeShowCallbacks)
+            callback(this);
+
         this._event.target.removeEventListener("contextmenu", this, true);
         InspectorFrontendHost.showContextMenu(event, this._menuObject);
         this._menuObject = null;
index 39f7943..d9a1ee8 100644 (file)
@@ -160,9 +160,9 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
         let breakpointNavigationBar = new WI.NavigationBar;
         breakpointNavigationBarWrapper.appendChild(breakpointNavigationBar.element);
 
-        let createBreakpointButton = new WI.ButtonNavigationItem("create-breakpoint", WI.UIString("Create Breakpoint"), "Images/Plus13.svg", 13, 13);
-        createBreakpointButton.addEventListener(WI.ButtonNavigationItem.Event.Clicked, this._handleCreateBreakpointClicked, this);
-        breakpointNavigationBar.addNavigationItem(createBreakpointButton);
+        this._createBreakpointButton = new WI.ButtonNavigationItem("create-breakpoint", WI.UIString("Create Breakpoint"), "Images/Plus13.svg", 13, 13);
+        this._createBreakpointButton.element.addEventListener("mousedown", this._handleCreateBreakpointMouseDown.bind(this));
+        breakpointNavigationBar.addNavigationItem(this._createBreakpointButton);
 
         let breakpointsGroup = new WI.DetailsSectionGroup([breakpointsRow]);
         let breakpointsSection = new WI.DetailsSection("breakpoints", WI.UIString("Breakpoints"), [breakpointsGroup], breakpointNavigationBarWrapper);
@@ -1416,9 +1416,17 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
             setting.value = !!treeElement.parent;
     }
 
-    _handleCreateBreakpointClicked(event)
+    _handleCreateBreakpointMouseDown(event)
     {
-        let contextMenu = WI.ContextMenu.createFromEvent(event.data.nativeEvent);
+        if (this._ignoreCreateBreakpointMouseDown)
+            return;
+
+        this._ignoreCreateBreakpointMouseDown = true;
+
+        let contextMenu = WI.ContextMenu.createFromEvent(event);
+        contextMenu.addBeforeShowCallback(() => {
+            this._ignoreCreateBreakpointMouseDown = false;
+        });
 
         // COMPATIBILITY (iOS 10): DebuggerAgent.setPauseOnAssertions did not exist yet.
         if (InspectorBackend.domains.Debugger.setPauseOnAssertions) {
@@ -1439,7 +1447,7 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
 
             contextMenu.appendItem(WI.UIString("Event Breakpoint\u2026"), () => {
                 let popover = new WI.EventBreakpointPopover(this);
-                popover.show(event.target.element, [WI.RectEdge.MAX_Y, WI.RectEdge.MIN_Y, WI.RectEdge.MAX_X]);
+                popover.show(this._createBreakpointButton.element, [WI.RectEdge.MAX_Y, WI.RectEdge.MIN_Y, WI.RectEdge.MAX_X]);
             });
 
             contextMenu.appendSeparator();
@@ -1457,7 +1465,7 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
 
             contextMenu.appendItem(WI.DOMDebuggerManager.supportsURLBreakpoints() ? WI.UIString("URL Breakpoint\u2026") : WI.UIString("XHR Breakpoint\u2026"), () => {
                 let popover = new WI.URLBreakpointPopover(this);
-                popover.show(event.target.element, [WI.RectEdge.MAX_Y, WI.RectEdge.MIN_Y, WI.RectEdge.MAX_X]);
+                popover.show(this._createBreakpointButton.element, [WI.RectEdge.MAX_Y, WI.RectEdge.MIN_Y, WI.RectEdge.MAX_X]);
             });
         }
 
index 0c403f2..8cbbbb7 100644 (file)
@@ -608,9 +608,21 @@ WI.LegacyTabBar = class LegacyTabBar extends WI.View
             if (!this._hiddenTabBarItems.length)
                 return;
 
+            if (this._ignoreTabPickerMouseDown)
+                return;
+
+            this._ignoreTabPickerMouseDown = true;
+
             let contextMenu = WI.ContextMenu.createFromEvent(event);
-            for (let item of this._hiddenTabBarItems)
-                contextMenu.appendItem(item.title, () => this.selectedTabBarItem = item);
+            contextMenu.addBeforeShowCallback(() => {
+                this._ignoreTabPickerMouseDown = false;
+            });
+
+            for (let item of this._hiddenTabBarItems) {
+                contextMenu.appendItem(item.title, () => {
+                    this.selectedTabBarItem = item;
+                });
+            }
 
             contextMenu.show();
             return;
index 99e60c5..b76d76c 100644 (file)
@@ -167,9 +167,9 @@ WI.SourcesNavigationSidebarPanel = class SourcesNavigationSidebarPanel extends W
         let breakpointNavigationBar = new WI.NavigationBar;
         breakpointNavigationBarWrapper.appendChild(breakpointNavigationBar.element);
 
-        let createBreakpointButton = new WI.ButtonNavigationItem("create-breakpoint", WI.UIString("Create Breakpoint"), "Images/Plus13.svg", 13, 13);
-        createBreakpointButton.addEventListener(WI.ButtonNavigationItem.Event.Clicked, this._handleCreateBreakpointClicked, this);
-        breakpointNavigationBar.addNavigationItem(createBreakpointButton);
+        this._createBreakpointButton = new WI.ButtonNavigationItem("create-breakpoint", WI.UIString("Create Breakpoint"), "Images/Plus13.svg", 13, 13);
+        this._createBreakpointButton.element.addEventListener("mousedown", this._handleCreateBreakpointMouseDown.bind(this));
+        breakpointNavigationBar.addNavigationItem(this._createBreakpointButton);
 
         let breakpointsGroup = new WI.DetailsSectionGroup([breakpointsRow]);
         let breakpointsSection = new WI.DetailsSection("breakpoints", WI.UIString("Breakpoints"), [breakpointsGroup], breakpointNavigationBarWrapper);
@@ -1331,9 +1331,17 @@ WI.SourcesNavigationSidebarPanel = class SourcesNavigationSidebarPanel extends W
             setting.value = !!treeElement.parent;
     }
 
-    _handleCreateBreakpointClicked(event)
+    _handleCreateBreakpointMouseDown(event)
     {
-        let contextMenu = WI.ContextMenu.createFromEvent(event.data.nativeEvent);
+        if (this._ignoreCreateBreakpointMouseDown)
+            return;
+
+        this._ignoreCreateBreakpointMouseDown = true;
+
+        let contextMenu = WI.ContextMenu.createFromEvent(event);
+        contextMenu.addBeforeShowCallback(() => {
+            this._ignoreCreateBreakpointMouseDown = false;
+        });
 
         // COMPATIBILITY (iOS 10): DebuggerAgent.setPauseOnAssertions did not exist yet.
         if (InspectorBackend.domains.Debugger.setPauseOnAssertions) {
@@ -1354,7 +1362,7 @@ WI.SourcesNavigationSidebarPanel = class SourcesNavigationSidebarPanel extends W
 
             contextMenu.appendItem(WI.UIString("Event Breakpoint\u2026"), () => {
                 let popover = new WI.EventBreakpointPopover(this);
-                popover.show(event.target.element, [WI.RectEdge.MAX_Y, WI.RectEdge.MIN_Y, WI.RectEdge.MAX_X]);
+                popover.show(this._createBreakpointButton.element, [WI.RectEdge.MAX_Y, WI.RectEdge.MIN_Y, WI.RectEdge.MAX_X]);
             });
 
             contextMenu.appendSeparator();
@@ -1372,7 +1380,7 @@ WI.SourcesNavigationSidebarPanel = class SourcesNavigationSidebarPanel extends W
 
             contextMenu.appendItem(WI.UIString("URL Breakpoint\u2026"), () => {
                 let popover = new WI.URLBreakpointPopover(this);
-                popover.show(event.target.element, [WI.RectEdge.MAX_Y, WI.RectEdge.MIN_Y, WI.RectEdge.MAX_X]);
+                popover.show(this._createBreakpointButton.element, [WI.RectEdge.MAX_Y, WI.RectEdge.MIN_Y, WI.RectEdge.MAX_X]);
             });
         }
 
index 259c26a..c9ef06d 100644 (file)
@@ -573,9 +573,21 @@ WI.TabBar = class TabBar extends WI.View
             if (!this._hiddenTabBarItems.length)
                 return;
 
+            if (this._ignoreTabPickerMouseDown)
+                return;
+
+            this._ignoreTabPickerMouseDown = true;
+
             let contextMenu = WI.ContextMenu.createFromEvent(event);
-            for (let item of this._hiddenTabBarItems)
-                contextMenu.appendItem(item.title, () => this.selectedTabBarItem = item);
+            contextMenu.addBeforeShowCallback(() => {
+                this._ignoreTabPickerMouseDown = false;
+            });
+
+            for (let item of this._hiddenTabBarItems) {
+                contextMenu.appendItem(item.title, () => {
+                    this.selectedTabBarItem = item;
+                });
+            }
 
             contextMenu.show();
             return;