Commit 19870fb3 authored by Erik Luo's avatar Erik Luo Committed by Commit Bot

DevTools: fix style tooltips after core/css refactor

DevTools agents rely on getting specific computed styles via
getComputedCSSValueInternal(). After a refactoring [1],
this method stopped accepting platform property names as
strings, and started using IDs.

This CL migrates those callsites to use IDs, which fixes
the broken style tooltips on hover during 'inspect mode'.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1676647

Bug: 982396
Change-Id: Iebe59024659a8ee90db1396cfb9b83796a9689b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1694404Reviewed-by: default avatarAndrey Kosyakov <caseq@chromium.org>
Reviewed-by: default avatarAnders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Erik Luo <luoe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676137}
parent 1958447b
...@@ -4754,6 +4754,8 @@ experimental domain Overlay ...@@ -4754,6 +4754,8 @@ experimental domain Overlay
DOM.NodeId nodeId DOM.NodeId nodeId
# Whether to include distance info. # Whether to include distance info.
optional boolean includeDistance optional boolean includeDistance
# Whether to include style info.
optional boolean includeStyle
returns returns
# Highlight data for the node. # Highlight data for the node.
object highlight object highlight
......
...@@ -201,9 +201,9 @@ void AppendStyleInfo(Node* node, ...@@ -201,9 +201,9 @@ void AppendStyleInfo(Node* node,
const InspectorHighlightContrastInfo& node_contrast) { const InspectorHighlightContrastInfo& node_contrast) {
std::unique_ptr<protocol::DictionaryValue> computed_style = std::unique_ptr<protocol::DictionaryValue> computed_style =
protocol::DictionaryValue::create(); protocol::DictionaryValue::create();
CSSStyleDeclaration* style = CSSComputedStyleDeclaration* style =
MakeGarbageCollected<CSSComputedStyleDeclaration>(node, true); MakeGarbageCollected<CSSComputedStyleDeclaration>(node, true);
Vector<AtomicString> properties; Vector<CSSPropertyID> properties;
// For text nodes, we can show color & font properties. // For text nodes, we can show color & font properties.
bool has_text_children = false; bool has_text_children = false;
...@@ -212,25 +212,26 @@ void AppendStyleInfo(Node* node, ...@@ -212,25 +212,26 @@ void AppendStyleInfo(Node* node,
has_text_children = child->IsTextNode(); has_text_children = child->IsTextNode();
} }
if (has_text_children) { if (has_text_children) {
properties.push_back("color"); properties.push_back(CSSPropertyID::kColor);
properties.push_back("font-family"); properties.push_back(CSSPropertyID::kFontFamily);
properties.push_back("font-size"); properties.push_back(CSSPropertyID::kFontSize);
properties.push_back("line-height"); properties.push_back(CSSPropertyID::kLineHeight);
} }
properties.push_back("padding"); properties.push_back(CSSPropertyID::kPadding);
properties.push_back("margin"); properties.push_back(CSSPropertyID::kMargin);
properties.push_back("background-color"); properties.push_back(CSSPropertyID::kBackgroundColor);
for (size_t i = 0; i < properties.size(); ++i) { for (size_t i = 0; i < properties.size(); ++i) {
const CSSValue* value = style->GetPropertyCSSValueInternal(properties[i]); const CSSValue* value = style->GetPropertyCSSValue(properties[i]);
if (!value) if (!value)
continue; continue;
AtomicString name = CSSPropertyName(properties[i]).ToAtomicString();
if (value->IsColorValue()) { if (value->IsColorValue()) {
Color color = static_cast<const cssvalue::CSSColorValue*>(value)->Value(); Color color = static_cast<const cssvalue::CSSColorValue*>(value)->Value();
computed_style->setString(properties[i], ToHEXA(color)); computed_style->setString(name, ToHEXA(color));
} else { } else {
computed_style->setString(properties[i], value->CssText()); computed_style->setString(name, value->CssText());
} }
} }
element_info->setValue("style", std::move(computed_style)); element_info->setValue("style", std::move(computed_style));
...@@ -479,11 +480,11 @@ void InspectorHighlight::AppendDistanceInfo(Node* node) { ...@@ -479,11 +480,11 @@ void InspectorHighlight::AppendDistanceInfo(Node* node) {
if (!layout_object) if (!layout_object)
return; return;
CSSStyleDeclaration* style = CSSComputedStyleDeclaration* style =
MakeGarbageCollected<CSSComputedStyleDeclaration>(node, true); MakeGarbageCollected<CSSComputedStyleDeclaration>(node, true);
for (size_t i = 0; i < style->length(); ++i) { for (size_t i = 0; i < style->length(); ++i) {
AtomicString name(style->item(i)); AtomicString name(style->item(i));
const CSSValue* value = style->GetPropertyCSSValueInternal(name); const CSSValue* value = style->GetPropertyCSSValue(cssPropertyID(name));
if (!value) if (!value)
continue; continue;
if (value->IsColorValue()) { if (value->IsColorValue()) {
......
...@@ -619,6 +619,7 @@ Response InspectorOverlayAgent::hideHighlight() { ...@@ -619,6 +619,7 @@ Response InspectorOverlayAgent::hideHighlight() {
Response InspectorOverlayAgent::getHighlightObjectForTest( Response InspectorOverlayAgent::getHighlightObjectForTest(
int node_id, int node_id,
Maybe<bool> include_distance, Maybe<bool> include_distance,
Maybe<bool> include_style,
std::unique_ptr<protocol::DictionaryValue>* result) { std::unique_ptr<protocol::DictionaryValue>* result) {
Node* node = nullptr; Node* node = nullptr;
Response response = dom_agent_->AssertNode(node_id, node); Response response = dom_agent_->AssertNode(node_id, node);
...@@ -634,10 +635,12 @@ Response InspectorOverlayAgent::getHighlightObjectForTest( ...@@ -634,10 +635,12 @@ Response InspectorOverlayAgent::getHighlightObjectForTest(
is_locked_ancestor = true; is_locked_ancestor = true;
} }
InspectorHighlight highlight( InspectorHighlightConfig config = InspectorHighlight::DefaultConfig();
node, InspectorHighlight::DefaultConfig(), config.show_styles = include_style.fromMaybe(false);
InspectorHighlightContrastInfo(), true /* append_element_info */, InspectorHighlight highlight(node, config, InspectorHighlightContrastInfo(),
include_distance.fromMaybe(false), is_locked_ancestor); true /* append_element_info */,
include_distance.fromMaybe(false),
is_locked_ancestor);
*result = highlight.AsProtocolValue(); *result = highlight.AsProtocolValue();
return Response::OK(); return Response::OK();
} }
......
...@@ -158,6 +158,7 @@ class CORE_EXPORT InspectorOverlayAgent final ...@@ -158,6 +158,7 @@ class CORE_EXPORT InspectorOverlayAgent final
protocol::Response getHighlightObjectForTest( protocol::Response getHighlightObjectForTest(
int node_id, int node_id,
protocol::Maybe<bool> include_distance, protocol::Maybe<bool> include_distance,
protocol::Maybe<bool> include_style,
std::unique_ptr<protocol::DictionaryValue>* highlight) override; std::unique_ptr<protocol::DictionaryValue>* highlight) override;
// InspectorBaseAgent overrides. // InspectorBaseAgent overrides.
......
...@@ -1155,6 +1155,19 @@ ElementsTestRunner.dumpInspectorDistanceJSON = function(idValue, callback) { ...@@ -1155,6 +1155,19 @@ ElementsTestRunner.dumpInspectorDistanceJSON = function(idValue, callback) {
} }
}; };
ElementsTestRunner.dumpInspectorHighlightStyleJSON = async function(idValue) {
const node = await ElementsTestRunner.nodeWithIdPromise(idValue);
const result = await TestRunner.OverlayAgent.getHighlightObjectForTest(node.id, false, true /* includeStyle */);
const info = result['elementInfo'] ? result['elementInfo']['style'] : null;
if (!info) {
TestRunner.addResult(`${idValue}: No style info`);
} else {
if (info['font-family'])
info['font-family'] = '<font-family value>';
TestRunner.addResult(idValue + JSON.stringify(info, null, 2));
}
};
ElementsTestRunner.waitForAnimationAdded = function(callback) { ElementsTestRunner.waitForAnimationAdded = function(callback) {
TestRunner.addSniffer(Animation.AnimationTimeline.prototype, '_addAnimationGroup', callback); TestRunner.addSniffer(Animation.AnimationTimeline.prototype, '_addAnimationGroup', callback);
}; };
......
This test verifies the style info overlaid on an inspected node.
empty-div{
"padding": "0px",
"margin": "0px",
"background-color": "#0000FFFF"
}
div-with-text{
"color": "#FF0000FF",
"font-family": "<font-family value>",
"font-size": "20px",
"line-height": "normal",
"padding": "0px",
"margin": "0px",
"background-color": "#0000FFFF"
}
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
(async function() {
TestRunner.addResult(`This test verifies the style info overlaid on an inspected node.\n`);
await TestRunner.loadModule('elements_test_runner');
await TestRunner.showPanel('elements');
await TestRunner.loadHTML(`
<style>
div {
color: red;
background-color: blue;
font-size: 20px;
}
</style>
<div id="empty-div"></div>
<div id="div-with-text">I have text</div>
`);
await ElementsTestRunner.dumpInspectorHighlightStyleJSON('empty-div');
await ElementsTestRunner.dumpInspectorHighlightStyleJSON('div-with-text');
TestRunner.completeTest();
})();
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment