- Revision
- 221993
- Author
- [email protected]
- Date
- 2017-09-13 13:48:09 -0700 (Wed, 13 Sep 2017)
Log Message
Web Inspector: Frontend should be made to expect and handle disabled properties
https://bugs.webkit.org/show_bug.cgi?id=166787
<rdar://problem/34379593>
Reviewed by Joseph Pecoraro.
Source/WebCore:
Include disabled (commented out) CSS properties in the payload.
Tests: inspector/css/css-property.html
inspector/css/matched-style-properties.html
* inspector/InspectorStyleSheet.cpp:
(WebCore::InspectorStyle::populateAllProperties const):
(WebCore::InspectorStyle::styleWithProperties const):
Source/WebInspectorUI:
This change introduces WI.CSSStyleDeclaration.prototype.allProperties getter,
that includes both enabled and disabled (commented out) CSS properties.
The existing WI.CSSStyleDeclaration.prototype.properties getter only includes enabled CSS properties,
same as before the backend change.
There is no behaviour change in the current styles sidebar. The new redesigned styles sidebar will
use disabled properties and display them as commented out.
* UserInterface/Models/CSSProperty.js:
(WI.CSSProperty.prototype.get attached):
Rename `enabled` to `attached`, as it didn't correspond to `_enabled` property. Attached means that the property
is enabled and has a ownerStyle with a set index (unless it's a computed style, where index is not applicable).
(WI.CSSProperty.prototype.get enabled):
* UserInterface/Models/CSSStyleDeclaration.js:
(WI.CSSStyleDeclaration.prototype.get allProperties):
Add allProperties getter that will be used in the new styles sidebar.
* UserInterface/Models/DOMNodeStyles.js:
(WI.DOMNodeStyles.prototype._markOverriddenProperties):
* UserInterface/Views/CSSStyleDeclarationTextEditor.js:
(WI.CSSStyleDeclarationTextEditor.prototype.highlightProperty.propertiesMatch):
Rename "enabled" to "attached" without any behavior change.
LayoutTests:
Add test cases for disabled (commented out) CSS properties.
* inspector/css/css-property-expected.txt:
* inspector/css/css-property.html:
Add test cases for previously untested `enabled` and newly added `attached` getters.
* inspector/css/matched-style-properties.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (221992 => 221993)
--- trunk/LayoutTests/ChangeLog 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/LayoutTests/ChangeLog 2017-09-13 20:48:09 UTC (rev 221993)
@@ -1,3 +1,19 @@
+2017-09-13 Nikita Vasilyev <[email protected]>
+
+ Web Inspector: Frontend should be made to expect and handle disabled properties
+ https://bugs.webkit.org/show_bug.cgi?id=166787
+ <rdar://problem/34379593>
+
+ Reviewed by Joseph Pecoraro.
+
+ Add test cases for disabled (commented out) CSS properties.
+
+ * inspector/css/css-property-expected.txt:
+ * inspector/css/css-property.html:
+ Add test cases for previously untested `enabled` and newly added `attached` getters.
+
+ * inspector/css/matched-style-properties.html:
+
2017-09-13 Alicia Boya García <[email protected]>
[GTK] Test gardening
Modified: trunk/LayoutTests/inspector/css/css-property-expected.txt (221992 => 221993)
--- trunk/LayoutTests/inspector/css/css-property-expected.txt 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/LayoutTests/inspector/css/css-property-expected.txt 2017-09-13 20:48:09 UTC (rev 221993)
@@ -26,6 +26,20 @@
PASS: "background-repeat-invalid" has the value "repeat".
PASS: "background-repeat-y" has the value "repeat".
+-- Running test case: CSSProperty.prototype.get enabled
+PASS: "background-repeat" is enabled.
+PASS: "background-repeat-x" is enabled.
+PASS: "background-repeat-invalid" is enabled.
+PASS: "background-color" is disabled.
+PASS: "background-repeat-y" is enabled.
+
+-- Running test case: CSSProperty.prototype.get attached
+PASS: "background-repeat" is attached.
+PASS: "background-repeat-x" is attached.
+PASS: "background-repeat-invalid" is attached.
+PASS: "background-color" is detached.
+PASS: "background-repeat-y" is attached.
+
-- Running test case: CSSProperty.prototype.get text
PASS: "background-repeat" has the text "background-repeat: repeat;".
PASS: "background-repeat" has the _text (private) "background-repeat: repeat;".
Modified: trunk/LayoutTests/inspector/css/css-property.html (221992 => 221993)
--- trunk/LayoutTests/inspector/css/css-property.html 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/LayoutTests/inspector/css/css-property.html 2017-09-13 20:48:09 UTC (rev 221993)
@@ -111,6 +111,60 @@
});
suite.addTestCase({
+ name: "CSSProperty.prototype.get enabled",
+ description: "Ensure valid, invalid, and internal-only have correct enabled state.",
+ test(resolve, reject) {
+ for (let rule of nodeStyles.matchedRules) {
+ if (rule.selectorText !== "div#x")
+ continue;
+
+ for (let property of rule.style.allProperties) {
+ switch (property.name) {
+ case "background-repeat":
+ case "background-repeat-x":
+ case "background-repeat-y":
+ case "background-repeat-invalid":
+ InspectorTest.expectThat(property.enabled, `"${property.name}" is enabled.`);
+ break;
+ case "background-color":
+ InspectorTest.expectThat(!property.enabled, `"${property.name}" is disabled.`);
+ break;
+ }
+ }
+ }
+
+ resolve();
+ }
+ });
+
+ suite.addTestCase({
+ name: "CSSProperty.prototype.get attached",
+ description: "Ensure valid, invalid, and internal-only have correct attached state.",
+ test(resolve, reject) {
+ for (let rule of nodeStyles.matchedRules) {
+ if (rule.selectorText !== "div#x")
+ continue;
+
+ for (let property of rule.style.allProperties) {
+ switch (property.name) {
+ case "background-repeat":
+ case "background-repeat-x":
+ case "background-repeat-y":
+ case "background-repeat-invalid":
+ InspectorTest.expectThat(property.attached, `"${property.name}" is attached.`);
+ break;
+ case "background-color":
+ InspectorTest.expectThat(!property.attached, `"${property.name}" is detached.`);
+ break;
+ }
+ }
+ }
+
+ resolve();
+ }
+ });
+
+ suite.addTestCase({
name: "CSSProperty.prototype.get text",
description: "Ensure valid, invalid, and internal-only have correct text.",
test(resolve, reject) {
@@ -177,6 +231,9 @@
background-repeat: repeat;
background-repeat-x: repeat;
background-repeat-invalid: repeat;
+ /* background-color: black; */
+ /* Not a CSS property */
+ /* foo:bar; foo:baz; */
}
</style>
<div id="x"></div>
Modified: trunk/LayoutTests/inspector/css/matched-style-properties.html (221992 => 221993)
--- trunk/LayoutTests/inspector/css/matched-style-properties.html 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/LayoutTests/inspector/css/matched-style-properties.html 2017-09-13 20:48:09 UTC (rev 221993)
@@ -10,6 +10,8 @@
position:absolute;
ToP:0;
lEfT:0;
+ /* font-size: 12px; */
+ /*float: left;*/
}
</style>
<script src=""
Modified: trunk/Source/WebCore/ChangeLog (221992 => 221993)
--- trunk/Source/WebCore/ChangeLog 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/Source/WebCore/ChangeLog 2017-09-13 20:48:09 UTC (rev 221993)
@@ -1,3 +1,20 @@
+2017-09-13 Nikita Vasilyev <[email protected]>
+
+ Web Inspector: Frontend should be made to expect and handle disabled properties
+ https://bugs.webkit.org/show_bug.cgi?id=166787
+ <rdar://problem/34379593>
+
+ Reviewed by Joseph Pecoraro.
+
+ Include disabled (commented out) CSS properties in the payload.
+
+ Tests: inspector/css/css-property.html
+ inspector/css/matched-style-properties.html
+
+ * inspector/InspectorStyleSheet.cpp:
+ (WebCore::InspectorStyle::populateAllProperties const):
+ (WebCore::InspectorStyle::styleWithProperties const):
+
2017-09-13 Carlos Alberto Lopez Perez <[email protected]>
[GTK] Fails to build because 'Float32Array' has not been declared in AudioContext.h
Modified: trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp (221992 => 221993)
--- trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/Source/WebCore/inspector/InspectorStyleSheet.cpp 2017-09-13 20:48:09 UTC (rev 221993)
@@ -601,9 +601,6 @@
ASSERT(!styleDeclarationOrException.hasException());
String styleDeclaration = styleDeclarationOrException.hasException() ? emptyString() : styleDeclarationOrException.releaseReturnValue();
for (auto& sourceData : *sourcePropertyData) {
- // FIXME: <https://webkit.org/b/166787> Web Inspector: Frontend should be made to expect and handle disabled properties
- if (sourceData.disabled)
- continue;
InspectorStyleProperty p(sourceData, true, sourceData.disabled);
p.setRawTextFromStyleDeclaration(styleDeclaration);
result->append(p);
@@ -663,17 +660,21 @@
// Default "priority" == "".
if (propertyEntry.important)
property->setPriority("important");
+
+ if (it->hasSource) {
+ // The property range is relative to the style body start.
+ // Should be converted into an absolute range (relative to the stylesheet start)
+ // for the proper conversion into line:column.
+ SourceRange absolutePropertyRange = propertyEntry.range;
+ absolutePropertyRange.start += ruleBodyRangeStart;
+ absolutePropertyRange.end += ruleBodyRangeStart;
+ property->setRange(buildSourceRangeObject(absolutePropertyRange, lineEndings.get()));
+ }
+
if (!it->disabled) {
if (it->hasSource) {
ASSERT(sourceData);
property->setImplicit(false);
- // The property range is relative to the style body start.
- // Should be converted into an absolute range (relative to the stylesheet start)
- // for the proper conversion into line:column.
- SourceRange absolutePropertyRange = propertyEntry.range;
- absolutePropertyRange.start += ruleBodyRangeStart;
- absolutePropertyRange.end += ruleBodyRangeStart;
- property->setRange(buildSourceRangeObject(absolutePropertyRange, lineEndings.get()));
// Parsed property overrides any property with the same name. Non-parsed property overrides
// previous non-parsed property with the same name (if any).
Modified: trunk/Source/WebInspectorUI/ChangeLog (221992 => 221993)
--- trunk/Source/WebInspectorUI/ChangeLog 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/Source/WebInspectorUI/ChangeLog 2017-09-13 20:48:09 UTC (rev 221993)
@@ -1,3 +1,36 @@
+2017-09-13 Nikita Vasilyev <[email protected]>
+
+ Web Inspector: Frontend should be made to expect and handle disabled properties
+ https://bugs.webkit.org/show_bug.cgi?id=166787
+ <rdar://problem/34379593>
+
+ Reviewed by Joseph Pecoraro.
+
+ This change introduces WI.CSSStyleDeclaration.prototype.allProperties getter,
+ that includes both enabled and disabled (commented out) CSS properties.
+
+ The existing WI.CSSStyleDeclaration.prototype.properties getter only includes enabled CSS properties,
+ same as before the backend change.
+
+ There is no behaviour change in the current styles sidebar. The new redesigned styles sidebar will
+ use disabled properties and display them as commented out.
+
+ * UserInterface/Models/CSSProperty.js:
+ (WI.CSSProperty.prototype.get attached):
+ Rename `enabled` to `attached`, as it didn't correspond to `_enabled` property. Attached means that the property
+ is enabled and has a ownerStyle with a set index (unless it's a computed style, where index is not applicable).
+
+ (WI.CSSProperty.prototype.get enabled):
+ * UserInterface/Models/CSSStyleDeclaration.js:
+ (WI.CSSStyleDeclaration.prototype.get allProperties):
+ Add allProperties getter that will be used in the new styles sidebar.
+
+ * UserInterface/Models/DOMNodeStyles.js:
+ (WI.DOMNodeStyles.prototype._markOverriddenProperties):
+ * UserInterface/Views/CSSStyleDeclarationTextEditor.js:
+ (WI.CSSStyleDeclarationTextEditor.prototype.highlightProperty.propertiesMatch):
+ Rename "enabled" to "attached" without any behavior change.
+
2017-09-13 Joseph Pecoraro <[email protected]>
Web Inspector: Escape in global search field should clear it
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js (221992 => 221993)
--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js 2017-09-13 20:48:09 UTC (rev 221993)
@@ -154,11 +154,14 @@
get priority() { return this._priority; }
- get enabled()
+ get attached()
{
return this._enabled && this._ownerStyle && (!isNaN(this._index) || this._ownerStyle.type === WI.CSSStyleDeclaration.Type.Computed);
}
+ // Only commented out properties are disabled.
+ get enabled() { return this._enabled; }
+
get overridden() { return this._overridden; }
set overridden(overridden)
{
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js (221992 => 221993)
--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js 2017-09-13 20:48:09 UTC (rev 221993)
@@ -99,7 +99,9 @@
var oldText = this._text;
this._text = text;
- this._properties = properties;
+ this._properties = properties.filter((property) => property.enabled);
+ this._allProperties = properties;
+
this._styleSheetTextRange = styleSheetTextRange;
this._propertyNameMap = {};
@@ -217,6 +219,8 @@
return this._properties;
}
+ get allProperties() { return this._allProperties; }
+
get visibleProperties()
{
if (this._visibleProperties)
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js (221992 => 221993)
--- trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/DOMNodeStyles.js 2017-09-13 20:48:09 UTC (rev 221993)
@@ -901,7 +901,7 @@
for (var j = 0; j < properties.length; ++j) {
var property = properties[j];
- if (!property.enabled || !property.valid) {
+ if (!property.attached || !property.valid) {
property.overridden = false;
continue;
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js (221992 => 221993)
--- trunk/Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js 2017-09-13 20:43:59 UTC (rev 221992)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js 2017-09-13 20:48:09 UTC (rev 221993)
@@ -189,7 +189,7 @@
{
function propertiesMatch(cssProperty)
{
- if (cssProperty.enabled && !cssProperty.overridden) {
+ if (cssProperty.attached && !cssProperty.overridden) {
if (cssProperty.canonicalName === property.canonicalName || hasMatchingLonghandProperty(cssProperty))
return true;
}
@@ -989,7 +989,7 @@
if (!this._codeMirror.getOption("readOnly")) {
// Create a new checkbox element and marker.
- console.assert(property.enabled);
+ console.assert(property.attached);
var checkboxElement = document.createElement("input");
checkboxElement.type = "checkbox";
@@ -1047,7 +1047,7 @@
else if (!property.valid && (!propertyNameIsValid || duplicatePropertyExistsBelow.call(this, property)))
classNames.push("invalid");
- if (!property.enabled)
+ if (!property.attached)
classNames.push("disabled");
if (property.__filterResultClassName && !property.__filterResultNeedlePosition)
