Bug 189597 – Wrong 'border' serialization with both common and uncommon values

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Oriol Brufau]

Run this code:

```js
element.style.cssText = "border: 5px solid blue; border-top: 5px dotted green";
element.style.border;
```

Expected: empty string, because the 'border' shorthand can't express that combination.
Result: "5px", because that's a common value for the 4 sides. The style and color have uncommon values so they are just omitted.

[Oriol Brufau]

Created attachment 349763 [details]
Patch

[Oriol Brufau]

Created attachment 349767 [details]
Patch

[EWS Watchlist]

Comment on attachment 349767 [details]
Patch

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

New failing tests:
fast/dom/css-shorthand-common-value.html
fast/css/getPropertyValue-border.html

[EWS Watchlist]

Created attachment 349773 [details]
Archive of layout-test-results from ews102 for mac-sierra

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

[EWS Watchlist]

Comment on attachment 349767 [details]
Patch

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

New failing tests:
fast/dom/css-shorthand-common-value.html
fast/css/getPropertyValue-border.html

[EWS Watchlist]

Created attachment 349777 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 349767 [details]
Patch

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

New failing tests:
fast/css/getPropertyValue-border.html
fast/dom/css-shorthand-common-value.html

[EWS Watchlist]

Created attachment 349780 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 349767 [details]
Patch

Attachment 349767 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9217469

New failing tests:
fast/css/getPropertyValue-border.html
fast/dom/css-shorthand-common-value.html

[EWS Watchlist]

Created attachment 349785 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[Simon Fraser (smfr)]

Comment on attachment 349767 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Remove CommonValueMode enum and make borderPropertyValue always return null
> +        when there are uncommon values (the previous ReturnNullOnUncommonValues mode).

Why was CommonValueMode added in the first place, and what's the justification for removing it?

[EWS Watchlist]

Comment on attachment 349767 [details]
Patch

Attachment 349767 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/9217908

New failing tests:
fast/dom/css-shorthand-common-value.html
fast/css/getPropertyValue-border.html

[EWS Watchlist]

Created attachment 349794 [details]
Archive of layout-test-results from ews203 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews203  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[EWS Watchlist]

Comment on attachment 349767 [details]
Patch

Attachment 349767 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/9218796

New failing tests:
fast/dom/css-shorthand-common-value.html
fast/css/getPropertyValue-border.html

[EWS Watchlist]

Created attachment 349797 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Oriol Brufau]

Created attachment 350250 [details]
Patch

[Oriol Brufau]

(In reply to Simon Fraser (smfr) from comment #11)
> Why was CommonValueMode added in the first place, and what's the
> justification for removing it?

Initially, there was no CommonValueMode and the behavior was always like OmitUncommonValues. This produced a wrong output both for cssText and getPropertyValue("border").

Then bug 83026 added CommonValueMode to fix the cssText behavior, but preserving the getPropertyValue("border") one.

But it's still wrong: https://drafts.csswg.org/cssom/#serialize-a-css-value
> If there is no such shorthand or shorthand cannot exactly represent the values of all the properties in list, return the empty string.

So now I want to remove CommonValueMode and always behave like ReturnNullOnUncommonValues.

[WebKit Commit Bot]

Comment on attachment 350250 [details]
Patch

Clearing flags on attachment: 350250

Committed r236278: <https://trac.webkit.org/changeset/236278>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44653562>