Similar to Bug 228697, we should replace webkit- prefix properties with Standard Properties in Layout/
<rdar://problem/81729542>
Created attachment 435558 [details] Patch
Created attachment 435559 [details] Patch
Comment on attachment 435559 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435559&action=review Thanks Sonia. It looks that you are close to get EWS green. Some comments inline... > LayoutTests/ChangeLog:144 > + * platform/gtk/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt: As a reminder, you should not modify any test in LayoutTests/imported/w3c/web-platform-tests/ (these have not been sync). Since you don't modify any code here in this patch, there is no need to update the expectation. > LayoutTests/fast/css/remove-shorthand-expected.txt:57 > +removes "margin-block-start-collapse, margin-block-end-collapse" You didn't modify remove-shorthand.html in this patch. It looks like this test is checking what are the longhand properties removed when removing the shorthand -webkit-margin-collapse. > LayoutTests/platform/glib/svg/css/getComputedStyle-basic-expected.txt:468 > +rect: style.getPropertyCSSValue(margin-block-start-collapse) : [object CSSPrimitiveValue] For all these getComputedStyle-basic-expected.txt, you modified the expectation but not the test LayoutTests/svg/css/getComputedStyle-basic.xhtml itself. I believe we want to continue to test the -webkit prefixed properties here, until they are actually removed. What you should do instead is updating Layout/fast/css/getComputedStyle/resources/property-names.js to add the unprefixed forms. Please check all the tests that are relying on this helper JS file and run locally before uploading a new patch: LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer.html LayoutTests/fast/css/getComputedStyle/computed-style.html LayoutTests/svg/css/getComputedStyle-basic.xhtml > LayoutTests/platform/ios/fast/css/getComputedStyle/computed-style-expected.txt:236 > +margin-block-start-collapse: collapse; For these computed-style-expected.txt files, see my comment about property-names.js > LayoutTests/platform/ios/fast/css/getComputedStyle/computed-style-without-renderer-expected.txt:235 > +margin-block-start-collapse: collapse For these computed-style-without-renderer-expected.txt files, see my comment about property-names.js
Created attachment 436106 [details] Patch
Comment on attachment 436106 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=436106&action=review LGTM, but probably wait that Oriol or Rego are back to comment on this. > LayoutTests/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=228730 I think it would be nice to have a short statement explaining what we are doing in this patch. In particular, the details of the old/new property names.
Created attachment 436290 [details] Patch
Comment on attachment 436290 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=436290&action=review Nice work, thanks for the patch. Just some comments inline. > LayoutTests/ChangeLog:32 > + Reviewed by NOBODY (OOPS!). Nit: This line is usually before the description of the change. > LayoutTests/ChangeLog:56 > + * fast/css/border-start-end.html: This test is specifically testing the -webkit- prefixed property, so it doesn't make sense to change it. Please keep it as it is. There are more like this: margin-start-end.html, padding-start-end.html.
Created attachment 437013 [details] Patch
Committed r281908 (241220@main): <https://commits.webkit.org/241220@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 437013 [details].