Attachment 370536 Details for Bug 198088 – Patch

[patch] Patch

bug-198088-20190523180620.patch (text/plain), 18.73 KB, created by Devin Rousso on 2019-05-23 18:06:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-05-23 18:06:20 PDT

Size: 18.73 KB

patch

obsolete

>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 25d6b03f5d7d74b4b987cdd036f9034e64b953bd..e9d34b8bafd14c1a66aab6c66f8d603fe17c128d 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -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
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 15abffaf65d903c19cf4949a678548444d8bf494..1bae1ee5fb6e1d0ca3b6fb5733fcbbb4d228f10c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,40 @@
>+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
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 8205b91b69492a9581fa52d2ad13c2b14a94c0c5..59eb51bd4709c7d1c1b40adb43b9853f40a4192c 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -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  Matt Baker  <mattbaker@apple.com>
> 
>         Web Inspector: Remove unused CSS class "offset-sections"
>diff --git a/Source/JavaScriptCore/inspector/protocol/DOM.json b/Source/JavaScriptCore/inspector/protocol/DOM.json
>index 77910c9b4c82edb760fd92646fdd4f7271211d00..b3674e7a0f8e6b02cce8876c4d37e53d44259958 100644
>--- a/Source/JavaScriptCore/inspector/protocol/DOM.json
>+++ b/Source/JavaScriptCore/inspector/protocol/DOM.json
>@@ -400,7 +400,8 @@
>             "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." }
>             ]
>         },
>         {
>diff --git a/Source/WebCore/inspector/InspectorOverlay.cpp b/Source/WebCore/inspector/InspectorOverlay.cpp
>index 007b61789368e94b3d94e0c443dd450efb2de429..09cbf6564181eed5b37c9fd43ad64a1640acadd3 100644
>--- a/Source/WebCore/inspector/InspectorOverlay.cpp
>+++ b/Source/WebCore/inspector/InspectorOverlay.cpp
>@@ -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();
>diff --git a/Source/WebCore/inspector/InspectorOverlay.h b/Source/WebCore/inspector/InspectorOverlay.h
>index 82800f064128a0708d4e3fbbe2d32e5c630af540..a9a1457e3952befb4fdeb626e03b0501c447a693 100644
>--- a/Source/WebCore/inspector/InspectorOverlay.h
>+++ b/Source/WebCore/inspector/InspectorOverlay.h
>@@ -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
>diff --git a/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp b/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
>index f345f15b7ae5bf6cc45823add29e4b67e4ac6700..91fda46fc1fef3083694fd274409b0a2d2757b3c 100644
>--- a/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
>+++ b/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
>@@ -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)
>diff --git a/Source/WebCore/inspector/agents/InspectorDOMAgent.h b/Source/WebCore/inspector/agents/InspectorDOMAgent.h
>index cb59b81ab6fd341116450050e11b10eba05e7848..58d3a20767f1fd7a1e395180c3b19ca749948506 100644
>--- a/Source/WebCore/inspector/agents/InspectorDOMAgent.h
>+++ b/Source/WebCore/inspector/agents/InspectorDOMAgent.h
>@@ -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.
>diff --git a/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js b/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
>index f97c188cb31128dbbd5caf1e8c8afd6f44c4f53d..e056d18c6cd09e831cdb312b43fef97554b4898d 100644
>--- a/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
>+++ b/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js
>@@ -372,6 +372,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";
>@@ -983,6 +984,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)";
>diff --git a/Source/WebInspectorUI/UserInterface/Base/Setting.js b/Source/WebInspectorUI/UserInterface/Base/Setting.js
>index f27691a8c6e77c401e2e270aa01309fef777f9d5..bfa983806359e22083863b7fe6dc0e6fde2a50d3 100644
>--- a/Source/WebInspectorUI/UserInterface/Base/Setting.js
>+++ b/Source/WebInspectorUI/UserInterface/Base/Setting.js
>@@ -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),
>diff --git a/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js b/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js
>index 7a91b3fea37385eab2dc1f80cb1f134cdc91bb89..96a6a228813160a8db9572f102d2eac1203edfee 100644
>--- a/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js
>+++ b/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js
>@@ -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);
>         });
>diff --git a/Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js b/Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js
>index 9646a26e80afdfa64b8ee803bcc2a8bee8e5e268..3ec95633c12421e81edd6f0d50ceaa4afc8701a9 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js
>@@ -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)]);
>

Actions: View | Formatted Diff | Diff

Attachments on bug 198088: 370488 | 370535 | 370536