| Summary: | REGRESSION (r263729): Carousel freezes on "fourth page"/fourth click on right arrow on netflix.com | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||
| Component: | Animations | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | dino, graouts, nicolekinser21, simon.fraser, thorton, webkit-bug-importer, wengshenshun | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Wenson Hsieh
2020-08-19 10:35:46 PDT
Created attachment 406954 [details]
Patch
Comment on attachment 406954 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406954&action=review > Source/WebCore/ChangeLog:11 > + `translate` CSS properties to a container `div`, and adds a `transitionend` event listener which the page did you mean `transition`? Comment on attachment 406954 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406954&action=review >> Source/WebCore/ChangeLog:11 >> + `translate` CSS properties to a container `div`, and adds a `transitionend` event listener which the page > > did you mean `transition`? Whoops, yes I did! Fixed. Comment on attachment 406954 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406954&action=review > Source/WebCore/ChangeLog:11 > + `translate` CSS properties to a container `div`, and adds a `transitionend` event listener which the page Did you mean `translate` here, or `transition`? > LayoutTests/animations/animation-followed-by-two-transitions.html:45 > + await UIHelper.delayFor(60); Why 60ms? Comment on attachment 406954 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406954&action=review >> LayoutTests/animations/animation-followed-by-two-transitions.html:45 >> + await UIHelper.delayFor(60); > > Why 60ms? So when I originally wrote this, I noticed that some delay was needed to cause the test to fail without the change in this patch. After some experimentation I thought that I needed to match the duration of the transition that is ending (60ms), but now after more digging, I realized that this is not the case. In order for the test to exercise the bug, I need to wait long enough here for the CSSTransition that was created as a result of adding the "transition" class to complete (i.e. `DocumentTimeline::transitionDidComplete`). It seems this call happens during the next rendering update after `AnimationTimeline::updateCSSAnimationsForElement`, which happens during the next rendering update after replacing the "transition" class with "no-transition". As such, it should be sufficient to replace this hard-coded delay with two `await UIHelper.renderingUpdate();`s. Created attachment 406969 [details]
Patch
Committed r265985: <https://trac.webkit.org/changeset/265985> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406969 [details]. *** Bug 215159 has been marked as a duplicate of this bug. *** *** Bug 217061 has been marked as a duplicate of this bug. *** |