RESOLVED FIXED 195914
[ContentChangeObserver] Add support for observing implicit transitions 
https://bugs.webkit.org/show_bug.cgi?id=195914
Summary [ContentChangeObserver] Add support for observing implicit transitions
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
DetailsFormatted DiffDiff
Patch (13.12 KB, patch)
2019-03-18 15:51 PDT, zalan 		no flags
DetailsFormatted DiffDiff
Patch (13.15 KB, patch)
2019-03-18 16:44 PDT, zalan 		no flags
DetailsFormatted DiffDiff
Patch (19.05 KB, patch)
2019-03-19 08:30 PDT, zalan 		no flags
DetailsFormatted DiffDiff
Patch (19.05 KB, patch)
2019-03-19 09:25 PDT, zalan 		no flags
DetailsFormatted DiffDiff
Patch (10.69 KB, patch)
2019-03-20 20:25 PDT, zalan 		no flags
DetailsFormatted DiffDiff
Patch (10.72 KB, patch)
2019-03-20 20:28 PDT, zalan 		simon.fraser: review+
DetailsFormatted DiffDiff
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>
zalan
Comment 10 2019-03-20 20:28:57 PDT
Created attachment 365471 [details] Patch
zalan
Comment 11 2019-03-21 11:33:16 PDT
Committed r243304: <https://trac.webkit.org/changeset/243304>
Note You need to log in before you can comment on or make changes to this bug.