Bug 231491

Summary:

Implement parsing and animation support for offset-distance, offset-position, offset-anchor

Product:

WebKit

Reporter:

Kiet Ho <kiet.ho>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

changseok, clopez, esprehn+autocc, ews-watchlist, glenn, graouts, gyuyoung.kim, joepeck, kiet.ho, kondapallykalyan, macpherson, menard, osvaldo.rivera1994, pdr, simon.fraser, webkit-bug-importer, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

203847

Attachments:

Description	Flags
Patch	ews-feeder: commit-queue-
Patch	ews-feeder: commit-queue-
Patch	none
WIP patch to see how many tests are still failing	none
WIP to see if the patch applies cleanly	none
WIP patch to make sure all tests are passing	none
WIP (again)	none
WIP (again) (again)	none
Patch for review	none
Patch	none
Patch for review	none

Kiet Ho

Reported 2021-10-10 13:20:16 PDT

Implement parsing support for offset-distance, offset-position, offset-anchor. offset-path support is planned for an upcoming patch.

Attachments
Patch (36.05 KB, patch) 2021-10-10 13:34 PDT, Kiet Ho	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (36.21 KB, patch) 2021-10-10 13:45 PDT, Kiet Ho	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (35.96 KB, patch) 2021-10-10 13:52 PDT, Kiet Ho	no flags	Details Formatted Diff Diff
WIP patch to see how many tests are still failing (134.29 KB, patch) 2021-10-12 02:34 PDT, Kiet Ho	no flags	Details Formatted Diff Diff
WIP to see if the patch applies cleanly (157.89 KB, patch) 2021-10-13 12:19 PDT, Kiet Ho	no flags	Details Formatted Diff Diff
WIP patch to make sure all tests are passing (322.94 KB, patch) 2021-10-13 18:07 PDT, Kiet Ho	no flags	Details Formatted Diff Diff
WIP (again) (356.95 KB, patch) 2021-10-13 21:32 PDT, Kiet Ho	no flags	Details Formatted Diff Diff
WIP (again) (again) (328.27 KB, patch) 2021-10-13 23:30 PDT, Kiet Ho	no flags	Details Formatted Diff Diff
Patch for review (328.62 KB, patch) 2021-10-14 11:19 PDT, Kiet Ho	no flags	Details Formatted Diff Diff
Patch (418.10 KB, patch) 2021-10-16 14:47 PDT, Kiet Ho	no flags	Details Formatted Diff Diff
Patch for review (418.05 KB, patch) 2021-10-16 15:48 PDT, Kiet Ho	no flags	Details Formatted Diff Diff
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.

Kiet Ho

Comment 1 2021-10-10 13:34:06 PDT

Created attachment 440733 [details] Patch

EWS Watchlist

Comment 2 2021-10-10 13:35:00 PDT

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

Kiet Ho

Comment 3 2021-10-10 13:45:09 PDT

Created attachment 440734 [details] Patch

Kiet Ho

Comment 4 2021-10-10 13:52:36 PDT

Created attachment 440736 [details] Patch

Simon Fraser (smfr)

Comment 5 2021-10-11 11:35:21 PDT

Comment on attachment 440736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440736&action=review Looks like you need to rebase more tests. > Source/WebCore/ChangeLog:8 > + No new tests; WPT parsing tests are sufficient. If bits of this patch are from external contributors, you should acknowledge them here. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3180 > + case CSSPropertyObjectPosition: > + return valueForPosition(style, style.objectPosition()); > + case CSSPropertyOffsetDistance: > + return cssValuePool.createValue(style.offsetDistance(), style); > + case CSSPropertyOffsetPosition: > + return valueForPositionOrAuto(style, style.offsetPosition()); > + case CSSPropertyOffsetAnchor: > + return valueForPositionOrAuto(style, style.offsetAnchor()); You can also use your helper for CSSPropertyPerspectiveOrigin, maybe others.

Kiet Ho

Comment 6 2021-10-12 02:24:13 PDT

Comment on attachment 440736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440736&action=review >> Source/WebCore/ChangeLog:8 >> + No new tests; WPT parsing tests are sufficient. > > If bits of this patch are from external contributors, you should acknowledge them here. I'll mention WPT tests that cover the implemented properties. >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3180 >> + return valueForPositionOrAuto(style, style.offsetAnchor()); > > You can also use your helper for CSSPropertyPerspectiveOrigin, maybe others. In the current code perspective-origin is parsed as a shorthand for some reason :/ If we ever change perspective-origin to use this helper, then it'd be for another patch.

Kiet Ho

Comment 7 2021-10-12 02:24:15 PDT

Kiet Ho

Comment 8 2021-10-12 02:25:13 PDT

(In reply to Simon Fraser (smfr) from comment #5) > Comment on attachment 440736 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=440736&action=review > > Looks like you need to rebase more tests. What do you mean by rebasing here? > > > Source/WebCore/ChangeLog:8 > > + No new tests; WPT parsing tests are sufficient. > > If bits of this patch are from external contributors, you should acknowledge > them here. > > > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3180 > > + case CSSPropertyObjectPosition: > > + return valueForPosition(style, style.objectPosition()); > > + case CSSPropertyOffsetDistance: > > + return cssValuePool.createValue(style.offsetDistance(), style); > > + case CSSPropertyOffsetPosition: > > + return valueForPositionOrAuto(style, style.offsetPosition()); > > + case CSSPropertyOffsetAnchor: > > + return valueForPositionOrAuto(style, style.offsetAnchor()); > > You can also use your helper for CSSPropertyPerspectiveOrigin, maybe others.

Kiet Ho

Comment 9 2021-10-12 02:34:01 PDT

Created attachment 440912 [details] WIP patch to see how many tests are still failing

Simon Fraser (smfr)

Comment 10 2021-10-12 09:22:12 PDT

"rebase" = re-run the tests to create new results, possibly with different results for different platforms. We call a platform result a "baseline", so "rebase" is to create new baselines.

Kiet Ho

Comment 11 2021-10-13 12:19:42 PDT

Created attachment 441119 [details] WIP to see if the patch applies cleanly

Kiet Ho

Comment 12 2021-10-13 18:07:52 PDT

Created attachment 441169 [details] WIP patch to make sure all tests are passing

Kiet Ho

Comment 13 2021-10-13 21:32:18 PDT

Created attachment 441183 [details] WIP (again)

Kiet Ho

Comment 14 2021-10-13 23:30:57 PDT

Created attachment 441186 [details] WIP (again) (again)

Kiet Ho

Comment 15 2021-10-14 11:19:07 PDT

Created attachment 441243 [details] Patch for review

Antoine Quint

Comment 16 2021-10-14 11:34:06 PDT

Comment on attachment 441243 [details] Patch for review View in context: https://bugs.webkit.org/attachment.cgi?id=441243&action=review Animation bits look good. > Source/WebCore/animation/CSSPropertyAnimation.cpp:919 > + bool canInterpolate(const RenderStyle& from, const RenderStyle& to) const override I don't know if it makes a different since the class is marked as `final` already, but this could be marked `final` as well.

Kiet Ho

Comment 17 2021-10-14 11:35:11 PDT

Comment on attachment 441243 [details] Patch for review View in context: https://bugs.webkit.org/attachment.cgi?id=441243&action=review >> Source/WebCore/animation/CSSPropertyAnimation.cpp:919 >> + bool canInterpolate(const RenderStyle& from, const RenderStyle& to) const override > > I don't know if it makes a different since the class is marked as `final` already, but this could be marked `final` as well. Gotcha, nice to know that.

Kiet Ho

Comment 18 2021-10-16 14:47:08 PDT

Created attachment 441505 [details] Patch

Kiet Ho

Comment 19 2021-10-16 15:48:33 PDT

Created attachment 441507 [details] Patch for review

Radar WebKit Bug Importer

Comment 20 2021-10-17 13:21:16 PDT

<rdar://problem/84350000>

EWS

Comment 21 2021-10-18 03:16:47 PDT

Committed r284361 (243146@main): <https://commits.webkit.org/243146@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441507 [details].

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