Title: [221691] trunk/Source/WebInspectorUI
Revision
221691
Author
commit-qu...@webkit.org
Date
2017-09-06 12:46:38 -0700 (Wed, 06 Sep 2017)

Log Message

Web Inspector: ⌘E and ⌘G do not work in main content area when quick console drawer is open
https://bugs.webkit.org/show_bug.cgi?id=176433

Patch by Joseph Pecoraro <pecor...@apple.com> on 2017-09-06
Reviewed by Brian Burg.

Make the ⌘E and ⌘G keyboard shortcuts behave more like the global ⌘K clear shortcut.
Instead of keeping them inside of FindBanner, and enabling/disabling them when they
make sense to apply, we make a single global instance of the keyboard shortcut and
apply it to whatever content view / content browser is active. This avoids conflicts
in situations where there are two ContentViews that are visible and each can handle
the keyboard shortcut.

* UserInterface/Base/Main.js:
(WI.contentLoaded):
New keyboard shortcuts.

(WI._populateFind):
(WI._findNext):
(WI._findPrevious):
Perform the shortcut on the active content view or content browser.

* UserInterface/Views/ContentBrowser.js:
(WI.ContentBrowser.prototype.shown):
(WI.ContentBrowser.prototype.hidden):
(WI.ContentBrowser.prototype.handlePopulateFindShortcut):
(WI.ContentBrowser.prototype.handleFindNextShortcut):
(WI.ContentBrowser.prototype.handleFindPreviousShortcut):
(WI.ContentBrowser.prototype.findBannerSearchQueryForSelection): Deleted.
* UserInterface/Views/LogContentView.js:
(WI.LogContentView.prototype.handlePopulateFindShortcut):
(WI.LogContentView.prototype.handleFindNextShortcut):
(WI.LogContentView.prototype.handleFindPreviousShortcut):
ContentBrowser and LogContentView (the only content view with a custom find banner)
now each handle the global keyboard shortcut handlers to do the right thing for
their respective cases.

* UserInterface/Views/FindBanner.js:
(WI.FindBanner.prototype.enableKeyboardShortcuts): Deleted.
(WI.FindBanner.prototype.disableKeyboardShortcuts): Deleted.
(WI.FindBanner.prototype._populateSearchQueryFromSelection): Deleted.
Move keyboard shortcut handling up to Main.js to be global.

* UserInterface/Views/ContentView.js:
(WI.ContentView.prototype.searchQueryWithSelection):
Default implementation should grab the selected text from the selection.
Previously shortcut selections only worked for selections in text editors.

Modified Paths

Diff

Modified: trunk/Source/WebInspectorUI/ChangeLog (221690 => 221691)


--- trunk/Source/WebInspectorUI/ChangeLog	2017-09-06 19:34:29 UTC (rev 221690)
+++ trunk/Source/WebInspectorUI/ChangeLog	2017-09-06 19:46:38 UTC (rev 221691)
@@ -1,3 +1,52 @@
+2017-09-06  Joseph Pecoraro  <pecor...@apple.com>
+
+        Web Inspector: ⌘E and ⌘G do not work in main content area when quick console drawer is open
+        https://bugs.webkit.org/show_bug.cgi?id=176433
+
+        Reviewed by Brian Burg.
+
+        Make the ⌘E and ⌘G keyboard shortcuts behave more like the global ⌘K clear shortcut.
+        Instead of keeping them inside of FindBanner, and enabling/disabling them when they
+        make sense to apply, we make a single global instance of the keyboard shortcut and
+        apply it to whatever content view / content browser is active. This avoids conflicts
+        in situations where there are two ContentViews that are visible and each can handle
+        the keyboard shortcut.
+
+        * UserInterface/Base/Main.js:
+        (WI.contentLoaded):
+        New keyboard shortcuts.
+
+        (WI._populateFind):
+        (WI._findNext):
+        (WI._findPrevious):
+        Perform the shortcut on the active content view or content browser.
+
+        * UserInterface/Views/ContentBrowser.js:
+        (WI.ContentBrowser.prototype.shown):
+        (WI.ContentBrowser.prototype.hidden):
+        (WI.ContentBrowser.prototype.handlePopulateFindShortcut):
+        (WI.ContentBrowser.prototype.handleFindNextShortcut):
+        (WI.ContentBrowser.prototype.handleFindPreviousShortcut):
+        (WI.ContentBrowser.prototype.findBannerSearchQueryForSelection): Deleted.
+        * UserInterface/Views/LogContentView.js:
+        (WI.LogContentView.prototype.handlePopulateFindShortcut):
+        (WI.LogContentView.prototype.handleFindNextShortcut):
+        (WI.LogContentView.prototype.handleFindPreviousShortcut):
+        ContentBrowser and LogContentView (the only content view with a custom find banner)
+        now each handle the global keyboard shortcut handlers to do the right thing for
+        their respective cases.
+
+        * UserInterface/Views/FindBanner.js:
+        (WI.FindBanner.prototype.enableKeyboardShortcuts): Deleted.
+        (WI.FindBanner.prototype.disableKeyboardShortcuts): Deleted.
+        (WI.FindBanner.prototype._populateSearchQueryFromSelection): Deleted.
+        Move keyboard shortcut handling up to Main.js to be global.
+
+        * UserInterface/Views/ContentView.js:
+        (WI.ContentView.prototype.searchQueryWithSelection):
+        Default implementation should grab the selected text from the selection.
+        Previously shortcut selections only worked for selections in text editors.
+
 2017-09-05  Nikita Vasilyev  <nvasil...@apple.com>
 
         Web Inspector: Styles Redesign: display "Inherited From" section headers

Modified: trunk/Source/WebInspectorUI/UserInterface/Base/Main.js (221690 => 221691)


--- trunk/Source/WebInspectorUI/UserInterface/Base/Main.js	2017-09-06 19:34:29 UTC (rev 221690)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/Main.js	2017-09-06 19:46:38 UTC (rev 221691)
@@ -288,6 +288,12 @@
 
     this.clearKeyboardShortcut = new WI.KeyboardShortcut(WI.KeyboardShortcut.Modifier.CommandOrControl, "K", this._clear.bind(this));
 
+    // FIXME: <https://webkit.org/b/151310> Web Inspector: Command-E should propagate to other search fields (including the system)
+    this.populateFindKeyboardShortcut = new WI.KeyboardShortcut(WI.KeyboardShortcut.Modifier.CommandOrControl, "E", this._populateFind.bind(this));
+    this.populateFindKeyboardShortcut.implicitlyPreventsDefault = false;
+    this.findNextKeyboardShortcut = new WI.KeyboardShortcut(WI.KeyboardShortcut.Modifier.CommandOrControl, "G", this._findNext.bind(this));
+    this.findPreviousKeyboardShortcut = new WI.KeyboardShortcut(WI.KeyboardShortcut.Modifier.Shift | WI.KeyboardShortcut.Modifier.CommandOrControl, "G", this._findPrevious.bind(this));
+
     this.quickConsole = new WI.QuickConsole(document.getElementById("quick-console"));
 
     this._consoleRepresentedObject = new WI.LogObject;
@@ -1975,6 +1981,51 @@
     contentView.handleClearShortcut(event);
 };
 
+WI._populateFind = function(event)
+{
+    let focusedContentView = this._focusedContentView();
+    if (focusedContentView.supportsCustomFindBanner) {
+        focusedContentView.handlePopulateFindShortcut();
+        return;
+    }
+
+    let contentBrowser = this._focusedOrVisibleContentBrowser();
+    if (!contentBrowser)
+        return;
+
+    contentBrowser.handlePopulateFindShortcut();
+}
+
+WI._findNext = function(event)
+{
+    let focusedContentView = this._focusedContentView();
+    if (focusedContentView.supportsCustomFindBanner) {
+        focusedContentView.handleFindNextShortcut();
+        return;
+    }
+
+    let contentBrowser = this._focusedOrVisibleContentBrowser();
+    if (!contentBrowser)
+        return;
+
+    contentBrowser.handleFindNextShortcut();
+}
+
+WI._findPrevious = function(event)
+{
+    let focusedContentView = this._focusedContentView();
+    if (focusedContentView.supportsCustomFindBanner) {
+        focusedContentView.handleFindPreviousShortcut();
+        return;
+    }
+
+    let contentBrowser = this._focusedOrVisibleContentBrowser();
+    if (!contentBrowser)
+        return;
+
+    contentBrowser.handleFindPreviousShortcut();
+}
+
 WI._copy = function(event)
 {
     var selection = window.getSelection();

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ContentBrowser.js (221690 => 221691)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ContentBrowser.js	2017-09-06 19:34:29 UTC (rev 221690)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ContentBrowser.js	2017-09-06 19:46:38 UTC (rev 221691)
@@ -227,65 +227,79 @@
         this._findBanner.show();
     }
 
-    findBannerPerformSearch(findBanner, query)
+    shown()
     {
-        var currentContentView = this.currentContentView;
-        if (!currentContentView || !currentContentView.supportsSearch)
-            return;
+        this._contentViewContainer.shown();
+    }
 
-        currentContentView.performSearch(query);
+    hidden()
+    {
+        this._contentViewContainer.hidden();
     }
 
-    findBannerSearchCleared(findBanner)
+    // Global ContentBrowser KeyboardShortcut handlers
+
+    handlePopulateFindShortcut()
     {
-        var currentContentView = this.currentContentView;
+        let currentContentView = this.currentContentView;
         if (!currentContentView || !currentContentView.supportsSearch)
             return;
 
-        currentContentView.searchCleared();
+        let searchQuery = currentContentView.searchQueryWithSelection();
+        if (!searchQuery)
+            return;
+
+        this._findBanner.searchQuery = searchQuery;
+
+        currentContentView.performSearch(this._findBanner.searchQuery);
     }
 
-    findBannerSearchQueryForSelection(findBanner)
+    handleFindNextShortcut()
     {
-        var currentContentView = this.currentContentView;
-        if (!currentContentView || !currentContentView.supportsSearch)
-            return null;
+        this.findBannerRevealNextResult(this._findBanner);
+    }
 
-        return currentContentView.searchQueryWithSelection();
+    handleFindPreviousShortcut()
+    {
+        this.findBannerRevealPreviousResult(this._findBanner);
     }
 
-    findBannerRevealPreviousResult(findBanner)
+    // FindBanner delegate
+
+    findBannerPerformSearch(findBanner, query)
     {
-        var currentContentView = this.currentContentView;
+        let currentContentView = this.currentContentView;
         if (!currentContentView || !currentContentView.supportsSearch)
             return;
 
-        currentContentView.revealPreviousSearchResult(!findBanner.showing);
+        currentContentView.performSearch(query);
     }
 
-    findBannerRevealNextResult(findBanner)
+    findBannerSearchCleared(findBanner)
     {
-        var currentContentView = this.currentContentView;
+        let currentContentView = this.currentContentView;
         if (!currentContentView || !currentContentView.supportsSearch)
             return;
 
-        currentContentView.revealNextSearchResult(!findBanner.showing);
+        currentContentView.searchCleared();
     }
 
-    shown()
+    findBannerRevealPreviousResult(findBanner)
     {
-        this._contentViewContainer.shown();
+        let currentContentView = this.currentContentView;
+        if (!currentContentView || !currentContentView.supportsSearch)
+            return;
 
-        if (this._findBanner)
-            this._findBanner.enableKeyboardShortcuts();
+        currentContentView.revealPreviousSearchResult(!findBanner.showing);
     }
 
-    hidden()
+    findBannerRevealNextResult(findBanner)
     {
-        this._contentViewContainer.hidden();
+        let currentContentView = this.currentContentView;
+        if (!currentContentView || !currentContentView.supportsSearch)
+            return;
 
-        if (this._findBanner)
-            this._findBanner.disableKeyboardShortcuts();
+        currentContentView.revealNextSearchResult(!findBanner.showing);
     }
 
     // Private

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ContentView.js (221690 => 221691)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ContentView.js	2017-09-06 19:34:29 UTC (rev 221690)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ContentView.js	2017-09-06 19:46:38 UTC (rev 221691)
@@ -449,8 +449,11 @@
 
     searchQueryWithSelection()
     {
-        // Implemented by subclasses.
-        return null;
+        let selection = window.getSelection();
+        if (selection.isCollapsed)
+            return null;
+
+        return selection.toString().removeWordBreakCharacters();
     }
 
     revealPreviousSearchResult(changeFocus)

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/FindBanner.js (221690 => 221691)


--- trunk/Source/WebInspectorUI/UserInterface/Views/FindBanner.js	2017-09-06 19:34:29 UTC (rev 221690)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/FindBanner.js	2017-09-06 19:46:38 UTC (rev 221691)
@@ -84,13 +84,6 @@
         this._searchBackwards = false;
         this._searchKeyPressed = false;
         this._previousSearchValue = "";
-
-        this._populateFindKeyboardShortcut = new WI.KeyboardShortcut(WI.KeyboardShortcut.Modifier.CommandOrControl, "E", this._populateSearchQueryFromSelection.bind(this));
-        this._populateFindKeyboardShortcut.implicitlyPreventsDefault = false;
-        this._findNextKeyboardShortcut = new WI.KeyboardShortcut(WI.KeyboardShortcut.Modifier.CommandOrControl, "G", this._nextResultButtonClicked.bind(this));
-        this._findPreviousKeyboardShortcut = new WI.KeyboardShortcut(WI.KeyboardShortcut.Modifier.Shift | WI.KeyboardShortcut.Modifier.CommandOrControl, "G", this._previousResultButtonClicked.bind(this));
-
-        this.disableKeyboardShortcuts();
     }
 
     // Public
@@ -223,20 +216,6 @@
         this.dispatchEventToListeners(WI.FindBanner.Event.DidHide);
     }
 
-    enableKeyboardShortcuts()
-    {
-        this._populateFindKeyboardShortcut.disabled = false;
-        this._findNextKeyboardShortcut.disabled = false;
-        this._findPreviousKeyboardShortcut.disabled = false;
-    }
-
-    disableKeyboardShortcuts()
-    {
-        this._populateFindKeyboardShortcut.disabled = true;
-        this._findNextKeyboardShortcut.disabled = true;
-        this._findPreviousKeyboardShortcut.disabled = true;
-    }
-
     // Private
 
     _inputFieldKeyDown(event)
@@ -279,19 +258,6 @@
         this._previousSearchValue = this.searchQuery;
     }
 
-    _populateSearchQueryFromSelection(event)
-    {
-        if (this._delegate && typeof this._delegate.findBannerSearchQueryForSelection === "function") {
-            var query = this._delegate.findBannerSearchQueryForSelection(this);
-            if (query) {
-                this.searchQuery = query;
-
-                if (this._delegate && typeof this._delegate.findBannerPerformSearch === "function")
-                    this._delegate.findBannerPerformSearch(this, this.searchQuery);
-            }
-        }
-    }
-
     _previousResultButtonClicked(event)
     {
         if (this._delegate && typeof this._delegate.findBannerRevealPreviousResult === "function")

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/LogContentView.js (221690 => 221691)


--- trunk/Source/WebInspectorUI/UserInterface/Views/LogContentView.js	2017-09-06 19:34:29 UTC (rev 221690)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/LogContentView.js	2017-09-06 19:46:38 UTC (rev 221691)
@@ -232,6 +232,27 @@
         this._logViewController.requestClearMessages();
     }
 
+    handlePopulateFindShortcut()
+    {
+        let searchQuery = this.searchQueryWithSelection();
+        if (!searchQuery)
+            return;
+
+        this._findBanner.searchQuery = searchQuery;
+
+        this.performSearch(this._findBanner.searchQuery);
+    }
+
+    handleFindNextShortcut()
+    {
+        this.findBannerRevealNextResult(this._findBanner);
+    }
+
+    handleFindPreviousShortcut()
+    {
+        this.findBannerRevealPreviousResult(this._findBanner);
+    }
+
     findBannerRevealPreviousResult()
     {
         this.highlightPreviousSearchMatch();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to