Web Inspector: Overlay: rulers/guides should be shown whenever element selection...
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 May 2019 01:34:32 +0000 (01:34 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 May 2019 01:34:32 +0000 (01:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198088

Reviewed by Timothy Hatcher.

When trying to "measure" the absolute position (to the viewport) or relative position (to
another element) of a given element, often the easiest way is to enable Element Selection
and Show Rulers at the same time.

This can have the undesired "side-effect" of having the rulers be always present, even when
not highlighting any nodes.

The ideal functionality is to allow the rulers/guides to be shown when element selection is
active and a node is hovered, regardless of whether "Show Rulers" is enabled.

Source/JavaScriptCore:

* inspector/protocol/DOM.json:
Add an optional `showRulers` parameter to `DOM.setInspectModeEnabled` that supersedes the
current value of `Page.setShowRulers` as to whether rulers/guides are shown.

Source/WebCore:

* inspector/InspectorOverlay.h:
(WebCore::InspectorOverlay::setShowRulersDuringElementSelection): Added.
* inspector/InspectorOverlay.cpp:
(WebCore::InspectorOverlay::paint):
(WebCore::InspectorOverlay::shouldShowOverlay):
(WebCore::InspectorOverlay::drawNodeHighlight):
(WebCore::InspectorOverlay::drawQuadHighlight):
(WebCore::InspectorOverlay::drawElementTitle):
If `showRulersDuringElementSelection` is enabled, draw rulers whenever any highlight bounds
are calculated, but don't update the overlay if it's the only thing enabled (e.g. if there's
no currently hovered node, the overlay will disappear).

* inspector/agents/InspectorDOMAgent.cpp:
(WebCore::InspectorDOMAgent::willDestroyFrontendAndBackend):
(WebCore::InspectorDOMAgent::inspect):
(WebCore::InspectorDOMAgent::setInspectModeEnabled):
(WebCore::InspectorDOMAgent::setSearchingForNode):
Add an optional `showRulers` parameter to `DOM.setInspectModeEnabled` that supersedes the
current value of `Page.setShowRulers` as to whether rulers/guides are shown.

Source/WebInspectorUI:

* UserInterface/Base/Setting.js:
* UserInterface/Views/SettingsTabContentView.js:
(WI.SettingsTabContentView.prototype._createGeneralSettingsView):
Add a setting for controlling whether rulers/guides are shown during element selection.

* UserInterface/Controllers/DOMManager.js:
(WI.DOMManager.prototype.set inspectModeEnabled):
Pass the setting value as an optional parameter when calling `DOM.setInspectModeEnabled`.

* Localizations/en.lproj/localizedStrings.js:

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

12 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/inspector/protocol/DOM.json
Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorOverlay.cpp
Source/WebCore/inspector/InspectorOverlay.h
Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
Source/WebCore/inspector/agents/InspectorDOMAgent.h
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
Source/WebInspectorUI/UserInterface/Base/Setting.js
Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js
Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js

index 25d6b03..e9d34b8 100644 (file)
@@ -1,3 +1,24 @@
+2019-05-23  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Overlay: rulers/guides should be shown whenever element selection is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=198088
+
+        Reviewed by Timothy Hatcher.
+
+        When trying to "measure" the absolute position (to the viewport) or relative position (to
+        another element) of a given element, often the easiest way is to enable Element Selection
+        and Show Rulers at the same time.
+
+        This can have the undesired "side-effect" of having the rulers be always present, even when
+        not highlighting any nodes.
+
+        The ideal functionality is to allow the rulers/guides to be shown when element selection is
+        active and a node is hovered, regardless of whether "Show Rulers" is enabled.
+
+        * inspector/protocol/DOM.json:
+        Add an optional `showRulers` parameter to `DOM.setInspectModeEnabled` that supersedes the
+        current value of `Page.setShowRulers` as to whether rulers/guides are shown.
+
 2019-05-23  Ross Kirsling  <ross.kirsling@sony.com>
 
         Socket-based RWI should be able to inspect a JSContext
index 77910c9..b3674e7 100644 (file)
             "description": "Enters the 'inspect' mode. In this mode, elements that user is hovering over are highlighted. Backend then generates 'inspect' command upon element selection.",
             "parameters": [
                 { "name": "enabled", "type": "boolean", "description": "True to enable inspection mode, false to disable it." },
-                { "name": "highlightConfig", "$ref": "HighlightConfig", "optional": true, "description": "A descriptor for the highlight appearance of hovered-over nodes. May be omitted if <code>enabled == false</code>." }
+                { "name": "highlightConfig", "$ref": "HighlightConfig", "optional": true, "description": "A descriptor for the highlight appearance of hovered-over nodes. May be omitted if <code>enabled == false</code>." },
+                { "name": "showRulers", "type": "boolean", "optional": true, "description": "Whether the rulers should be shown during element selection. This overrides Page.setShowRulers." }
             ]
         },
         {
index 15abffa..1bae1ee 100644 (file)
@@ -1,5 +1,42 @@
 2019-05-23  Devin Rousso  <drousso@apple.com>
 
+        Web Inspector: Overlay: rulers/guides should be shown whenever element selection is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=198088
+
+        Reviewed by Timothy Hatcher.
+
+        When trying to "measure" the absolute position (to the viewport) or relative position (to
+        another element) of a given element, often the easiest way is to enable Element Selection
+        and Show Rulers at the same time.
+
+        This can have the undesired "side-effect" of having the rulers be always present, even when
+        not highlighting any nodes.
+
+        The ideal functionality is to allow the rulers/guides to be shown when element selection is
+        active and a node is hovered, regardless of whether "Show Rulers" is enabled.
+
+        * inspector/InspectorOverlay.h:
+        (WebCore::InspectorOverlay::setShowRulersDuringElementSelection): Added.
+        * inspector/InspectorOverlay.cpp:
+        (WebCore::InspectorOverlay::paint):
+        (WebCore::InspectorOverlay::shouldShowOverlay):
+        (WebCore::InspectorOverlay::drawNodeHighlight):
+        (WebCore::InspectorOverlay::drawQuadHighlight):
+        (WebCore::InspectorOverlay::drawElementTitle):
+        If `showRulersDuringElementSelection` is enabled, draw rulers whenever any highlight bounds
+        are calculated, but don't update the overlay if it's the only thing enabled (e.g. if there's
+        no currently hovered node, the overlay will disappear).
+
+        * inspector/agents/InspectorDOMAgent.cpp:
+        (WebCore::InspectorDOMAgent::willDestroyFrontendAndBackend):
+        (WebCore::InspectorDOMAgent::inspect):
+        (WebCore::InspectorDOMAgent::setInspectModeEnabled):
+        (WebCore::InspectorDOMAgent::setSearchingForNode):
+        Add an optional `showRulers` parameter to `DOM.setInspectModeEnabled` that supersedes the
+        current value of `Page.setShowRulers` as to whether rulers/guides are shown.
+
+2019-05-23  Devin Rousso  <drousso@apple.com>
+
         Web Inspector: Overlay: rulers should switch sides if they intersect the highlighted node(s) so they don't obstruct any content
         https://bugs.webkit.org/show_bug.cgi?id=198165
 
index 007b617..09cbf65 100644 (file)
@@ -411,7 +411,7 @@ void InspectorOverlay::paint(GraphicsContext& context)
     if (!m_paintRects.isEmpty())
         drawPaintRects(context, m_paintRects);
 
-    if (m_showRulers)
+    if (m_showRulers || m_showRulersDuringElementSelection)
         drawRulers(context, bounds);
 }
 
@@ -492,6 +492,8 @@ void InspectorOverlay::setIndicating(bool indicating)
 
 bool InspectorOverlay::shouldShowOverlay() const
 {
+    // Don't show the overlay when m_showRulersDuringElementSelection is true, as it's only supposed
+    // to have an effect when element selection is active (e.g. a node is hovered).
     return m_highlightNode || m_highlightNodeList || m_highlightQuad || m_indicating || m_showPaintRects || m_showRulers;
 }
 
@@ -577,7 +579,7 @@ Highlight::Bounds InspectorOverlay::drawNodeHighlight(GraphicsContext& context,
     if (m_nodeHighlightConfig.showInfo)
         drawShapeHighlight(context, node, bounds);
 
-    if (m_showRulers)
+    if (m_showRulers || m_showRulersDuringElementSelection)
         drawBounds(context, bounds);
 
     // Ensure that the title information is drawn after the bounds.
@@ -597,7 +599,7 @@ Highlight::Bounds InspectorOverlay::drawQuadHighlight(GraphicsContext& context,
     if (highlight.quads.size() >= 1) {
         drawOutlinedQuad(context, highlight.quads[0], highlight.contentColor, highlight.contentOutlineColor, bounds);
 
-        if (m_showRulers)
+        if (m_showRulers || m_showRulersDuringElementSelection)
             drawBounds(context, bounds);
     }
 
@@ -929,7 +931,7 @@ void InspectorOverlay::drawElementTitle(GraphicsContext& context, Node& node, co
 
     FloatSize contentInset(0, pageView->topContentInset(ScrollView::TopContentInsetType::WebCoreOrPlatformContentInset));
     contentInset.expand(elementDataSpacing, elementDataSpacing);
-    if (m_showRulers)
+    if (m_showRulers || m_showRulersDuringElementSelection)
         contentInset.expand(rulerSize, rulerSize);
 
     float anchorTop = bounds.y();
index 82800f0..a9a1457 100644 (file)
@@ -115,6 +115,7 @@ public:
     void showPaintRect(const FloatRect&);
 
     void setShowRulers(bool);
+    void setShowRulersDuringElementSelection(bool enabled) { m_showRulersDuringElementSelection = enabled; }
 
     Node* highlightedNode() const;
 
@@ -150,9 +151,10 @@ private:
     Deque<TimeRectPair> m_paintRects;
     Timer m_paintRectUpdateTimer;
 
-    bool m_indicating {false};
-    bool m_showPaintRects {false};
-    bool m_showRulers {false};
+    bool m_indicating { false };
+    bool m_showPaintRects { false };
+    bool m_showRulers { false };
+    bool m_showRulersDuringElementSelection { false };
 };
 
 } // namespace WebCore
index f345f15..91fda46 100644 (file)
@@ -325,7 +325,7 @@ void InspectorDOMAgent::willDestroyFrontendAndBackend(Inspector::DisconnectReaso
     m_inspectedNode = nullptr;
 
     ErrorString unused;
-    setSearchingForNode(unused, false, nullptr);
+    setSearchingForNode(unused, false, nullptr, false);
     hideHighlight(unused);
 
     m_instrumentingAgents.setInspectorDOMAgent(nullptr);
@@ -1094,7 +1094,7 @@ void InspectorDOMAgent::inspect(Node* inspectedNode)
 {
     ErrorString unused;
     RefPtr<Node> node = inspectedNode;
-    setSearchingForNode(unused, false, nullptr);
+    setSearchingForNode(unused, false, nullptr, false);
 
     if (node->nodeType() != Node::ELEMENT_NODE && node->nodeType() != Node::DOCUMENT_NODE)
         node = node->parentNode();
@@ -1147,14 +1147,16 @@ void InspectorDOMAgent::highlightMousedOverNode()
         m_overlay->highlightNode(node, *m_inspectModeHighlightConfig);
 }
 
-void InspectorDOMAgent::setSearchingForNode(ErrorString& errorString, bool enabled, const JSON::Object* highlightInspectorObject)
+void InspectorDOMAgent::setSearchingForNode(ErrorString& errorString, bool enabled, const JSON::Object* highlightInspectorObject, bool showRulers)
 {
     if (m_searchingForNode == enabled)
         return;
 
     m_searchingForNode = enabled;
 
-    if (enabled) {
+    m_overlay->setShowRulersDuringElementSelection(m_searchingForNode && showRulers);
+
+    if (m_searchingForNode) {
         m_inspectModeHighlightConfig = highlightConfigFromInspectorObject(errorString, highlightInspectorObject);
         if (!m_inspectModeHighlightConfig)
             return;
@@ -1187,9 +1189,9 @@ std::unique_ptr<HighlightConfig> InspectorDOMAgent::highlightConfigFromInspector
     return highlightConfig;
 }
 
-void InspectorDOMAgent::setInspectModeEnabled(ErrorString& errorString, bool enabled, const JSON::Object* highlightConfig)
+void InspectorDOMAgent::setInspectModeEnabled(ErrorString& errorString, bool enabled, const JSON::Object* highlightConfig, const bool* showRulers)
 {
-    setSearchingForNode(errorString, enabled, highlightConfig ? highlightConfig : nullptr);
+    setSearchingForNode(errorString, enabled, highlightConfig ? highlightConfig : nullptr, showRulers && *showRulers);
 }
 
 void InspectorDOMAgent::highlightRect(ErrorString&, int x, int y, int width, int height, const JSON::Object* color, const JSON::Object* outlineColor, const bool* usePageCoordinates)
index cb59b81..58d3a20 100644 (file)
@@ -120,7 +120,7 @@ public:
     void discardSearchResults(ErrorString&, const String& searchId) override;
     void resolveNode(ErrorString&, int nodeId, const String* objectGroup, RefPtr<Inspector::Protocol::Runtime::RemoteObject>& result) override;
     void getAttributes(ErrorString&, int nodeId, RefPtr<JSON::ArrayOf<String>>& result) override;
-    void setInspectModeEnabled(ErrorString&, bool enabled, const JSON::Object* highlightConfig) override;
+    void setInspectModeEnabled(ErrorString&, bool enabled, const JSON::Object* highlightConfig, const bool* showRulers) override;
     void requestNode(ErrorString&, const String& objectId, int* nodeId) override;
     void pushNodeByPathToFrontend(ErrorString&, const String& path, int* nodeId) override;
     void hideHighlight(ErrorString&) override;
@@ -204,7 +204,7 @@ private:
 #endif
 
     void highlightMousedOverNode();
-    void setSearchingForNode(ErrorString&, bool enabled, const JSON::Object* highlightConfig);
+    void setSearchingForNode(ErrorString&, bool enabled, const JSON::Object* highlightConfig, bool showRulers);
     std::unique_ptr<HighlightConfig> highlightConfigFromInspectorObject(ErrorString&, const JSON::Object* highlightInspectorObject);
 
     // Node-related methods.
index fa30486..e3aa85b 100644 (file)
@@ -1,3 +1,31 @@
+2019-05-23  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Overlay: rulers/guides should be shown whenever element selection is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=198088
+
+        Reviewed by Timothy Hatcher.
+
+        When trying to "measure" the absolute position (to the viewport) or relative position (to
+        another element) of a given element, often the easiest way is to enable Element Selection
+        and Show Rulers at the same time.
+
+        This can have the undesired "side-effect" of having the rulers be always present, even when
+        not highlighting any nodes.
+
+        The ideal functionality is to allow the rulers/guides to be shown when element selection is
+        active and a node is hovered, regardless of whether "Show Rulers" is enabled.
+
+        * UserInterface/Base/Setting.js:
+        * UserInterface/Views/SettingsTabContentView.js:
+        (WI.SettingsTabContentView.prototype._createGeneralSettingsView):
+        Add a setting for controlling whether rulers/guides are shown during element selection.
+
+        * UserInterface/Controllers/DOMManager.js:
+        (WI.DOMManager.prototype.set inspectModeEnabled):
+        Pass the setting value as an optional parameter when calling `DOM.setInspectModeEnabled`.
+
+        * Localizations/en.lproj/localizedStrings.js:
+
 2019-05-23  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r245665.
index 5046616..910b908 100644 (file)
@@ -365,6 +365,7 @@ localizedStrings["Edit configuration"] = "Edit configuration";
 localizedStrings["Edit custom gradient"] = "Edit custom gradient";
 localizedStrings["Editing audits"] = "Editing audits";
 localizedStrings["Element"] = "Element";
+localizedStrings["Element Selection:"] = "Element Selection:";
 localizedStrings["Element clips compositing descendants"] = "Element clips compositing descendants";
 localizedStrings["Element has CSS blending applied and composited descendants"] = "Element has CSS blending applied and composited descendants";
 localizedStrings["Element has CSS filters applied"] = "Element has CSS filters applied";
@@ -957,6 +958,7 @@ localizedStrings["Show network information"] = "Show network information";
 localizedStrings["Show only for selected node"] = "Show only for selected node";
 localizedStrings["Show page load timing"] = "Show page load timing";
 localizedStrings["Show page resources"] = "Show page resources";
+localizedStrings["Show page rulers and node border lines"] = "Show page rulers and node border lines";
 localizedStrings["Show shadow DOM nodes"] = "Show shadow DOM nodes";
 localizedStrings["Show the details sidebar (%s)"] = "Show the details sidebar (%s)";
 localizedStrings["Show the navigation sidebar (%s)"] = "Show the navigation sidebar (%s)";
index f27691a..bfa9838 100644 (file)
@@ -166,6 +166,7 @@ WI.settings = {
     showJavaScriptTypeInformation: new WI.Setting("show-javascript-type-information", false),
     showPaintRects: new WI.Setting("show-paint-rects", false),
     showRulers: new WI.Setting("show-rulers", false),
+    showRulersDuringElementSelection: new WI.Setting("show-rulers-during-element-selection", true),
     showScopeChainOnPause: new WI.Setting("show-scope-chain-sidebar", true),
     showShadowDOM: new WI.Setting("show-shadow-dom", false),
     showWhitespaceCharacters: new WI.Setting("show-whitespace-characters", false),
index 7a91b3f..96a6a22 100644 (file)
@@ -540,7 +540,12 @@ WI.DOMManager = class DOMManager extends WI.Object
         if (enabled === this._inspectModeEnabled)
             return;
 
-        DOMAgent.setInspectModeEnabled(enabled, this._buildHighlightConfig(), (error) => {
+        let commandArguments = {
+            enabled,
+            highlightConfig: this._buildHighlightConfig(),
+            showRulers: WI.settings.showRulersDuringElementSelection.value,
+        };
+        DOMAgent.setInspectModeEnabled.invoke(commandArguments, (error) => {
             this._inspectModeEnabled = error ? false : enabled;
             this.dispatchEventToListeners(WI.DOMManager.Event.InspectModeStateChanged);
         });
index 9646a26..3ec9563 100644 (file)
@@ -222,6 +222,10 @@ WI.SettingsTabContentView = class SettingsTabContentView extends WI.TabContentVi
 
         generalSettingsView.addSeparator();
 
+        generalSettingsView.addSetting(WI.UIString("Element Selection:"), WI.settings.showRulersDuringElementSelection, WI.UIString("Show page rulers and node border lines"));
+
+        generalSettingsView.addSeparator();
+
         const zoomLevels = [0.6, 0.8, 1, 1.2, 1.4, 1.6, 1.8, 2, 2.2, 2.4];
         const zoomValues = zoomLevels.map((level) => [level, Number.percentageString(level, 0)]);