Bug 189597

Summary:

Wrong 'border' serialization with both common and uncommon values

Product:

WebKit

Reporter:

Oriol Brufau <obrufau>

Component:

CSS

Assignee:

Oriol Brufau <obrufau>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, ews-watchlist, koivisto, rego, rniwa, simon.fraser, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

188697

Attachments:

Description	Flags
Patch	none
Patch	none
Archive of layout-test-results from ews102 for mac-sierra	none
Archive of layout-test-results from ews107 for mac-sierra-wk2	none
Archive of layout-test-results from ews116 for mac-sierra	none
Archive of layout-test-results from ews125 for ios-simulator-wk2	none
Archive of layout-test-results from ews203 for win-future	none
Archive of layout-test-results from ews206 for win-future	none
Patch	none

Oriol Brufau

Reported 2018-09-13 11:47:04 PDT

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.

Attachments
Patch (8.01 KB, patch) 2018-09-14 08:52 PDT, Oriol Brufau	no flags	Details Formatted Diff Diff
Patch (8.98 KB, patch) 2018-09-14 09:35 PDT, Oriol Brufau	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-sierra (2.33 MB, application/zip) 2018-09-14 10:29 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.25 MB, application/zip) 2018-09-14 10:53 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews116 for mac-sierra (3.03 MB, application/zip) 2018-09-14 11:05 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.27 MB, application/zip) 2018-09-14 11:38 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews203 for win-future (12.84 MB, application/zip) 2018-09-14 13:20 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews206 for win-future (12.82 MB, application/zip) 2018-09-14 13:39 PDT, EWS Watchlist	no flags	Details
Patch (15.91 KB, patch) 2018-09-20 12:48 PDT, Oriol Brufau	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Oriol Brufau

Comment 1 2018-09-14 08:52:04 PDT

Created attachment 349763 [details] Patch

Oriol Brufau

Comment 2 2018-09-14 09:35:58 PDT

Created attachment 349767 [details] Patch

EWS Watchlist

Comment 3 2018-09-14 10:29:42 PDT

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

Comment 4 2018-09-14 10:29:44 PDT

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 5 2018-09-14 10:53:49 PDT

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

Comment 6 2018-09-14 10:53:50 PDT

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 7 2018-09-14 11:05:18 PDT

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

Comment 8 2018-09-14 11:05:20 PDT

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 9 2018-09-14 11:38:52 PDT

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

Comment 10 2018-09-14 11:38:54 PDT

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 11 2018-09-14 12:55:49 PDT

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 12 2018-09-14 13:20:11 PDT

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

Comment 13 2018-09-14 13:20:25 PDT

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 14 2018-09-14 13:39:16 PDT

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

Comment 15 2018-09-14 13:39:28 PDT

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

Comment 16 2018-09-20 12:48:32 PDT

Created attachment 350250 [details] Patch

Oriol Brufau

Comment 17 2018-09-20 12:58:56 PDT

(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 18 2018-09-20 13:43:44 PDT

Comment on attachment 350250 [details] Patch Clearing flags on attachment: 350250 Committed r236278: <https://trac.webkit.org/changeset/236278>

WebKit Commit Bot

Comment 19 2018-09-20 13:43:46 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 20 2018-09-20 13:44:38 PDT

<rdar://problem/44653562>

Note You need to log in before you can comment on or make changes to this bug.