RESOLVED FIXED 229083
Unprefix -webkit-text-align-last
https://bugs.webkit.org/show_bug.cgi?id=229083
Summary Unprefix -webkit-text-align-last
Sam Sneddon [:gsnedders]
Reported 2021-08-13 10:53:51 PDT
I don't think there's been much in the way of changes here, so this should be a simple change.
Attachments
Patch (29.43 KB, patch)
2021-12-10 14:25 PST, Brent Fulgham
ntim: review-
ews-feeder: commit-queue-
Radar WebKit Bug Importer
Comment 1 2021-08-20 10:54:22 PDT
Brent Fulgham
Comment 2 2021-12-09 16:58:20 PST
It looks like it's hidden behind ENABLE(CSS3_TEXT), so not sure it's even available for most people?
Brent Fulgham
Comment 3 2021-12-10 14:25:45 PST
Sam Sneddon [:gsnedders]
Comment 4 2021-12-10 14:57:26 PST
Comment on attachment 446814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446814&action=review Do we have any actual functional tests, above and beyond these tests that checking parsing/serialization? > LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/text-align-last-computed-expected.txt:9 > +FAIL Property text-align-last value 'match-parent' assert_equals: expected "match-parent" but got "auto" This seems bad? Is this just serialisation, or do we at least get the semantics right?
Brent Fulgham
Comment 5 2021-12-10 15:06:28 PST
Comment on attachment 446814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446814&action=review >> LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/text-align-last-computed-expected.txt:9 >> +FAIL Property text-align-last value 'match-parent' assert_equals: expected "match-parent" but got "auto" > > This seems bad? Is this just serialisation, or do we at least get the semantics right? I'm not sure if the test is correct. The spec says that 'match-parent' should match the immediate parent (which we don't have in this test), or the initial containing block which does not have a value specified for the direction value. That makes it sound like it should either be 'left', or maybe the default "auto" value. I don't think the computed value should ever be "match-parent".
Tim Nguyen (:ntim)
Comment 6 2021-12-10 16:33:11 PST
Comment on attachment 446814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446814&action=review >>> LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/text-align-last-computed-expected.txt:9 >>> +FAIL Property text-align-last value 'match-parent' assert_equals: expected "match-parent" but got "auto" >> >> This seems bad? Is this just serialisation, or do we at least get the semantics right? > > I'm not sure if the test is correct. The spec says that 'match-parent' should match the immediate parent (which we don't have in this test), or the initial containing block which does not have a value specified for the direction value. That makes it sound like it should either be 'left', or maybe the default "auto" value. I don't think the computed value should ever be "match-parent". See https://github.com/w3c/csswg-drafts/issues/2577 I don't think the outcome was particularly clear unfortunately. Might be worth pinging frivoal or fantasai.
Tim Nguyen (:ntim)
Comment 9 2021-12-10 16:41:12 PST
(In reply to Tim Nguyen (:ntim) from comment #7) > Comment on attachment 446814 [details] > Patch > > Do those WPTs start passing: > https://webkit-search.igalia.com/webkit/rev/ > c7aee49ebf38fcbd12c681b28cc2e877d356a1e9/LayoutTests/TestExpectations#2671- > 2673 > ? Looks like the WPT script adds the -webkit- prefix automatically to -webkit-text-align-last in those tests, can you please remove the now unused prefixes? See: https://webkit-search.igalia.com/webkit/search?q=-webkit-text-align-last&path=LayoutTests&case=false&regexp=false
Dean Jackson
Comment 11 2022-01-13 09:35:55 PST
Comment on attachment 446814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446814&action=review > Source/WebCore/rendering/style/RenderStyle.cpp:768 > -#if ENABLE(CSS3_TEXT) > || first.textAlignLast != second.textAlignLast > +#if ENABLE(CSS3_TEXT) Is text-align-last no longer part of CSS3 Text?
Razvan Caliman
Comment 12 2022-01-13 11:13:29 PST
Comment on attachment 446814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=446814&action=review > Source/WebInspectorUI/UserInterface/External/CSSDocumentation/CSSDocumentation.js:2360 > + "syntax": "auto | start | end | left | right | center | justify | match-parent", Please don't modify `CSSDocumentation.js` directly. It gets automatically generated from upstream data by running the script: `Source/WebInspectorUI/Scripts/update-inspector-css-documentation` To update a field, you can add an entry to `Source/WebInspectorUI/UserInterface/External/CSSDocumentation/CSSDocumentation-overrides.json` which will **overwrite** the corresponding field from the data source: ``` "text-align-last": { "syntax": "auto | start | end | left | right | center | justify | match-parent" } ``` Then run the script `Source/WebInspectorUI/Scripts/update-inspector-css-documentation` to generate the updated `CSSDocumentation.js` file. Alternatively, you can file an issue / send a PR to the upstream data source at https://github.com/microsoft/vscode-custom-data After it gets merged, on the next manual run of the script, the changes will get picked up automatically.
Antoine Quint
Comment 13 2022-03-08 08:42:48 PST
You should consider adding animation support for this property in CSSPropertyAnimation. See the many uses of DiscretePropertyWrapper, this will likely be a one-liner.
Tim Nguyen (:ntim)
Comment 14 2022-03-25 12:22:49 PDT
Comment on attachment 446814 [details] Patch r- to remove from queue given all the current comments + EWS status.
Tim Nguyen (:ntim)
Comment 15 2022-05-29 02:55:19 PDT
EWS
Comment 16 2022-05-30 01:29:50 PDT
Committed r295021 (251116@main): <https://commits.webkit.org/251116@main> Reviewed commits have been landed. Closing PR #1145 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.