Bug 194318 – Web Inspector: Styles: PropertiesChanged shouldn't fire when old and new text are both empty

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

inspector/css/modify-css-property-race.html is still failing on debug build (and debug build only!) because of this.
https://webkit-queues.webkit.org/results/11045743

Also, it adds an unnecessary entropy to the universe :)

[Nikita Vasilyev]

Created attachment 361249 [details]
Patch

[Devin Rousso]

Comment on attachment 361249 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361249&action=review

rs=me

> LayoutTests/inspector/css/modify-css-property-race.html:14
> +        let height = parseInt(element.style.height) + 1;

oops :P

> LayoutTests/inspector/css/modify-css-property-race.html:72
> +                    InspectorTest.expectGreaterThan(heightNumber, 11, "Height should be greater than 11px.");

Rather than compare the hardcoded 10 and 11, you could save a `previousHeightNumber` and ensure that the first one is greater than 10 and the second one is greater than `previousHeightNumber` (and change the text to say "Height should have increased from 10px" so you don't have to log the actual increased value).

> LayoutTests/inspector/css/modify-css-property-race.html:77
> +                    InspectorTest.expectGreaterThanOrEqual(heightNumber, 101, "Height should be greater or equal 101px.");

Ditto (72).

[EWS Watchlist]

Comment on attachment 361249 [details]
Patch

Attachment 361249 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11046952

New failing tests:
inspector/css/modify-css-property.html

[EWS Watchlist]

Created attachment 361260 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Nikita Vasilyev]

This broke other tests :(
Fix incoming.

[Nikita Vasilyev]

Created attachment 361262 [details]
Patch

[Nikita Vasilyev]

(In reply to Devin Rousso from comment #2)
> > LayoutTests/inspector/css/modify-css-property-race.html:72
> > +                    InspectorTest.expectGreaterThan(heightNumber, 11, "Height should be greater than 11px.");
> 
> Rather than compare the hardcoded 10 and 11, you could save a
> `previousHeightNumber` and ensure that the first one is greater than 10 and
> the second one is greater than `previousHeightNumber` (and change the text
> to say "Height should have increased from 10px" so you don't have to log the
> actual increased value).

I actually find it easier to follow when it's hardcoded there. However, I do like the less "robotic" assert messages!

[WebKit Commit Bot]

Comment on attachment 361262 [details]
Patch

Clearing flags on attachment: 361262

Committed r241011: <https://trac.webkit.org/changeset/241011>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/47842941>

[Truitt Savell]

After the fixes in https://trac.webkit.org/changeset/241011/webkit

inspector/css/modify-css-property-race.html

has become a flakey failure on Debug. Looks like it it somehow writing another pass message? 

History:
http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Fcss%2Fmodify-css-property-race.html

Diff:
--- /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/inspector/css/modify-css-property-race-expected.txt
+++ /Volumes/Data/slave/highsierra-debug-tests-wk1/build/layout-test-results/inspector/css/modify-css-property-race-actual.txt
@@ -7,4 +7,5 @@
 PASS: Height should have increased from 11px.
 PASS: Height should be 100px or more.
 PASS: Height should be 101px or more.
+PASS: Height should be 101px or more.