Bug 195914

Summary:

[ContentChangeObserver] Add support for observing implicit transitions

Product:

WebKit

Reporter:

zalan <zalan>

Component:

Layout and Rendering

Assignee:

zalan <zalan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, dino, dstockwell, ews-watchlist, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	simon.fraser: review+

zalan

Reported 2019-03-18 15:03:29 PDT

ssia

Attachments
Patch (13.12 KB, patch) 2019-03-18 15:09 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (13.12 KB, patch) 2019-03-18 15:51 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (13.15 KB, patch) 2019-03-18 16:44 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (19.05 KB, patch) 2019-03-19 08:30 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (19.05 KB, patch) 2019-03-19 09:25 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (10.69 KB, patch) 2019-03-20 20:25 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (10.72 KB, patch) 2019-03-20 20:28 PDT, zalan	simon.fraser: review+	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

zalan

Comment 1 2019-03-18 15:09:31 PDT

Created attachment 365073 [details] Patch

zalan

Comment 2 2019-03-18 15:51:09 PDT

Created attachment 365080 [details] Patch

zalan

Comment 3 2019-03-18 16:44:41 PDT

Created attachment 365089 [details] Patch

Simon Fraser (smfr)

Comment 4 2019-03-18 18:10:20 PDT

Comment on attachment 365089 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365089&action=review > Source/WebCore/page/ios/ContentChangeObserver.cpp:130 > +void ContentChangeObserver::didFinishAnimationOrTransition(const RenderElement& renderer) You can be running transition on multiple properties on an element. it might transition margin-left for 10ms, and left for 200ms. What happens then? Seems like you need to track per CSS property ID. You might also run one transition, then another animation, all with different properties. > Source/WebCore/rendering/RenderElement.cpp:2175 > + document().contentChangeObserver().didStartAnimationOrTransition(*this, propertyID, Seconds { timeOffset }); Is it a timeOffset or a duration? > Source/WebCore/rendering/RenderElement.cpp:2194 > + document().contentChangeObserver().didStartAnimationOrTransition(*this, animation->property(), Seconds { timeOffset }); Ditto. Is animation ever non-null?

zalan

Comment 5 2019-03-18 18:13:39 PDT

(In reply to Simon Fraser (smfr) from comment #4) > Comment on attachment 365089 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=365089&action=review > > > Source/WebCore/page/ios/ContentChangeObserver.cpp:130 > > +void ContentChangeObserver::didFinishAnimationOrTransition(const RenderElement& renderer) > > You can be running transition on multiple properties on an element. it might > transition margin-left for 10ms, and left for 200ms. What happens then? > Seems like you need to track per CSS property ID. > > You might also run one transition, then another animation, all with > different properties. We track only one property at this point.

zalan

Comment 6 2019-03-19 08:30:40 PDT

Created attachment 365162 [details] Patch

zalan

Comment 7 2019-03-19 09:25:03 PDT

Created attachment 365170 [details] Patch

zalan

Comment 8 2019-03-20 20:25:04 PDT

Created attachment 365470 [details] Patch

Radar WebKit Bug Importer

Comment 9 2019-03-20 20:27:03 PDT

<rdar://problem/49091959>