221961 2021-02-16 06:39:08 -0800 REGRESSION(r271515): ::marker fired at wrong time 2021-02-16 14:01:41 -0800 1 1 1 Unclassified WebKit Animations WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 gsnedders graouts dino graouts graouts webkit-bug-importer oldest_to_newest 1729658 0 gsnedders 2021-02-16 06:39:08 -0800 https://wpt.fyi/results/css/css-animations/event-order.tentative.html?q=seq%28status%3Apass%7Cstatus%3Aok%20status%3Apass%7Cstatus%3Aok%20status%3Apass%7Cstatus%3Aok%20status%3Apass%7Cstatus%3Aok%20status%3Apass%7Cstatus%3Aok%20status%3A%21pass%26status%3A%21ok%20status%3A%21pass%26status%3A%21ok%20status%3A%21pass%26status%3A%21ok%20status%3A%21pass%26status%3A%21ok%20status%3A%21pass%26status%3A%21ok%29&run_id=5703710288117760&run_id=5730791449427968&run_id=5176095785615360&run_id=5761234664161280&run_id=6289110253699072&run_id=6295692878282752&run_id=5717790667309056&run_id=6252961795670016&run_id=5112363554439168&run_id=5651017565732864 1729670 1 graouts 2021-02-16 07:46:26 -0800 Probably regressed with r271515, confirming this now. 1729681 2 graouts 2021-02-16 08:38:21 -0800 Indeed, it is, regressed due to r271515, the fix for bug 220652. 1729682 3 graouts 2021-02-16 08:39:55 -0800 Just looking at the very simple patch, the order in which pseudo-elements are generated is different since in the PseudoId enum the relative order is Marker, Before, After. While changing the order in which we process animations likely addresses this, I don't think this should be relevant and there is some sorting code somewhere that hasn't been updated for ::marker support. 1729696 4 graouts 2021-02-16 09:03:21 -0800 Indeed, this simple patch fixes the regression: diff --git a/Source/WebCore/style/StyleTreeResolver.cpp b/Source/WebCore/style/StyleTreeResolver.cpp index a8a107e6e6aa..6af316dadfa4 100644 --- a/Source/WebCore/style/StyleTreeResolver.cpp +++ b/Source/WebCore/style/StyleTreeResolver.cpp @@ -249,12 +249,12 @@ ElementUpdates TreeResolver::resolveElement(Element& element) } PseudoIdToElementUpdateMap pseudoUpdates; + if (auto markerElementUpdate = resolvePseudoStyle(element, update, PseudoId::Marker)) + pseudoUpdates.set(PseudoId::Marker, WTFMove(*markerElementUpdate)); if (auto beforeElementUpdate = resolvePseudoStyle(element, update, PseudoId::Before)) pseudoUpdates.set(PseudoId::Before, WTFMove(*beforeElementUpdate)); if (auto afterElementUpdate = resolvePseudoStyle(element, update, PseudoId::After)) pseudoUpdates.set(PseudoId::After, WTFMove(*afterElementUpdate)); - if (auto markerElementUpdate = resolvePseudoStyle(element, update, PseudoId::Marker)) - pseudoUpdates.set(PseudoId::Marker, WTFMove(*markerElementUpdate)); #if ENABLE(TOUCH_ACTION_REGIONS) // FIXME: Track this exactly. But I think this should be handled by the compareDeclarativeAnimationOwningElementPositionsInDocumentTreeOrder() utility. 1729700 5 graouts 2021-02-16 09:10:28 -0800 Oh, we simply don't invoke the composite order sorting code when dispatching events, in DocumentTimelinesController::updateAnimationsAndSendEvents(): // 6. Perform a stable sort of the animation events in events to dispatch as follows. std::stable_sort(events.begin(), events.end(), [] (const Ref<AnimationEventBase>& lhs, const Ref<AnimationEventBase>& rhs) { // 1. Sort the events by their scheduled event time such that events that were scheduled to occur earlier, sort before events scheduled to occur later // and events whose scheduled event time is unresolved sort before events with a resolved scheduled event time. // 2. Within events with equal scheduled event times, sort by their composite order. FIXME: Need to do this. return lhs->timelineTime() < rhs->timelineTime(); }); So this test progressed when we added support for ::marker, but in a very fragile way that just worked out of luck. 1729796 6 graouts 2021-02-16 10:49:41 -0800 That said, touching the event sorting code is a bit risky so I'll do the first fix which will restore functionality. 1729797 7 webkit-bug-importer 2021-02-16 10:50:11 -0800 <rdar://problem/74397846> 1729798 8 420499 graouts 2021-02-16 10:52:21 -0800 Created attachment 420499 Patch 1729925 9 ews-feeder 2021-02-16 14:01:39 -0800 Committed r272927: <https://commits.webkit.org/r272927> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420499. 420499 2021-02-16 10:52:21 -0800 2021-02-16 14:01:40 -0800 Patch bug-221961-20210216195220.patch text/plain 3792 graouts U3VidmVyc2lvbiBSZXZpc2lvbjogMjcyOTA0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOWNiMTg5ZmRiY2I1MjJm NDU5NTRjMGI0MGE5YmZmOTkzNTlmNTZlYy4uNzNiNGM2YjVhNjE3YmZkZTc4MTA5MTg5OGU1MzEx ODU1M2Q5ZmQ0NCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDIxLTAyLTE2ICBBbnRv aW5lIFF1aW50ICA8Z3Jhb3V0c0B3ZWJraXQub3JnPgorCisgICAgICAgIFJFR1JFU1NJT04ocjI3 MTUxNSk6IDo6bWFya2VyIGZpcmVkIGF0IHdyb25nIHRpbWUKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyMTk2MQorICAgICAgICA8cmRhcjovL3Byb2Js ZW0vNzQzOTc4NDY+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgUmVzdG9yZSB0aGUgb3JkZXIgaW4gd2hpY2ggd2UgZ2VuZXJhdGUgQ1NTIEFuaW1hdGlv bnMgZm9yIHBzZXVkby1lbGVtZW50cyBhcyBpdCB3YXMgcHJpb3IgdG8gcjI3MTUxNS4KKworICAg ICAgICAqIHN0eWxlL1N0eWxlVHJlZVJlc29sdmVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlN0 eWxlOjpUcmVlUmVzb2x2ZXI6OnJlc29sdmVFbGVtZW50KToKKwogMjAyMS0wMi0xNiAgQW50b2lu ZSBRdWludCAgPGdyYW91dHNAd2Via2l0Lm9yZz4KIAogICAgICAgICBXZWIgQW5pbWF0aW9ucyBK UyBBUEkgZG9lcyBub3Qgc3VwcG9ydCAiaW5oZXJpdCIgQ1NTIHZhbHVlcyBmb3Iga2V5ZnJhbWVz CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9zdHlsZS9TdHlsZVRyZWVSZXNvbHZlci5jcHAg Yi9Tb3VyY2UvV2ViQ29yZS9zdHlsZS9TdHlsZVRyZWVSZXNvbHZlci5jcHAKaW5kZXggYThhMTA3 ZTZlNmFhNjhiZmU3ODlkMGM0NjlmMTdhZWJlY2UxYjNjMC4uNmFmMzE2ZGFkZmE0N2Y0OThkMTVk OTg5MWRiMmVmZmNiMGNkYzQ0NyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvc3R5bGUvU3R5 bGVUcmVlUmVzb2x2ZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3N0eWxlL1N0eWxlVHJlZVJl c29sdmVyLmNwcApAQCAtMjQ5LDEyICsyNDksMTIgQEAgRWxlbWVudFVwZGF0ZXMgVHJlZVJlc29s dmVyOjpyZXNvbHZlRWxlbWVudChFbGVtZW50JiBlbGVtZW50KQogICAgIH0KIAogICAgIFBzZXVk b0lkVG9FbGVtZW50VXBkYXRlTWFwIHBzZXVkb1VwZGF0ZXM7CisgICAgaWYgKGF1dG8gbWFya2Vy RWxlbWVudFVwZGF0ZSA9IHJlc29sdmVQc2V1ZG9TdHlsZShlbGVtZW50LCB1cGRhdGUsIFBzZXVk b0lkOjpNYXJrZXIpKQorICAgICAgICBwc2V1ZG9VcGRhdGVzLnNldChQc2V1ZG9JZDo6TWFya2Vy LCBXVEZNb3ZlKCptYXJrZXJFbGVtZW50VXBkYXRlKSk7CiAgICAgaWYgKGF1dG8gYmVmb3JlRWxl bWVudFVwZGF0ZSA9IHJlc29sdmVQc2V1ZG9TdHlsZShlbGVtZW50LCB1cGRhdGUsIFBzZXVkb0lk OjpCZWZvcmUpKQogICAgICAgICBwc2V1ZG9VcGRhdGVzLnNldChQc2V1ZG9JZDo6QmVmb3JlLCBX VEZNb3ZlKCpiZWZvcmVFbGVtZW50VXBkYXRlKSk7CiAgICAgaWYgKGF1dG8gYWZ0ZXJFbGVtZW50 VXBkYXRlID0gcmVzb2x2ZVBzZXVkb1N0eWxlKGVsZW1lbnQsIHVwZGF0ZSwgUHNldWRvSWQ6OkFm dGVyKSkKICAgICAgICAgcHNldWRvVXBkYXRlcy5zZXQoUHNldWRvSWQ6OkFmdGVyLCBXVEZNb3Zl KCphZnRlckVsZW1lbnRVcGRhdGUpKTsKLSAgICBpZiAoYXV0byBtYXJrZXJFbGVtZW50VXBkYXRl ID0gcmVzb2x2ZVBzZXVkb1N0eWxlKGVsZW1lbnQsIHVwZGF0ZSwgUHNldWRvSWQ6Ok1hcmtlcikp Ci0gICAgICAgIHBzZXVkb1VwZGF0ZXMuc2V0KFBzZXVkb0lkOjpNYXJrZXIsIFdURk1vdmUoKm1h cmtlckVsZW1lbnRVcGRhdGUpKTsKIAogI2lmIEVOQUJMRShUT1VDSF9BQ1RJT05fUkVHSU9OUykK ICAgICAvLyBGSVhNRTogVHJhY2sgdGhpcyBleGFjdGx5LgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVz dHMvQ2hhbmdlTG9nIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCmluZGV4IGNkMGRjNGQ4NjM3NzQ3 OGNhNzYwZmMxYTg3YTEzOTljYjZlMTc4MjIuLmM3YWI0YWM2NmU5MTA0OTExMzY1OTViMTQyZTMw ZDRlN2I2MDI5YmUgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL0NoYW5nZUxvZworKysgYi9MYXlv dXRUZXN0cy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAyMS0wMi0xNiAgQW50b2luZSBR dWludCAgPGdyYW91dHNAd2Via2l0Lm9yZz4KKworICAgICAgICBSRUdSRVNTSU9OKHIyNzE1MTUp OiA6Om1hcmtlciBmaXJlZCBhdCB3cm9uZyB0aW1lCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMjE5NjEKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzc0 Mzk3ODQ2PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFJlbW92ZSBmbGFraW5lc3MgZm9yIHRoaXMgdGVzdCBzaW5jZSBpdCBkaWRuJ3QgYWxsb3cgdXMg dG8gc3BvdCB0aGUgcmVncmVzc2lvbiBpbnRyb2R1Y2VkIGJ5IHIyNzE1MTUuCisKKyAgICAgICAg KiBUZXN0RXhwZWN0YXRpb25zOgorCiAyMDIxLTAyLTE2ICBBbnRvaW5lIFF1aW50ICA8Z3Jhb3V0 c0B3ZWJraXQub3JnPgogCiAgICAgICAgIFdlYiBBbmltYXRpb25zIEpTIEFQSSBkb2VzIG5vdCBz dXBwb3J0ICJpbmhlcml0IiBDU1MgdmFsdWVzIGZvciBrZXlmcmFtZXMKZGlmZiAtLWdpdCBhL0xh eW91dFRlc3RzL1Rlc3RFeHBlY3RhdGlvbnMgYi9MYXlvdXRUZXN0cy9UZXN0RXhwZWN0YXRpb25z CmluZGV4IGUwMTUxNTQ1MDY1OTViNDZkMzZiYzI0MTIyZmVmMTA2NDY2ZDhkOWYuLmIxMGJlODYz OTBlZGNlNjk4Y2RjM2VjYjI5ZmYxZjliYjIzZjY3YmIgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3Rz L1Rlc3RFeHBlY3RhdGlvbnMKKysrIGIvTGF5b3V0VGVzdHMvVGVzdEV4cGVjdGF0aW9ucwpAQCAt MzE5NSw3ICszMTk1LDYgQEAgd2Via2l0Lm9yZy9iLzIwMDIwOSBpbXBvcnRlZC93M2Mvd2ViLXBs YXRmb3JtLXRlc3RzL2Nzcy9jc3MtaW1hZ2VzL211bHRpcGxlLXBvc2kKIAogIyB3cHQgY3NzLXRy YW5zaXRpb25zIGFuZCBjc3MtYW5pbWF0aW9ucyBmYWlsdXJlcyBhbmQgZmxha2luZXNzCiB3ZWJr aXQub3JnL2IvMjAzMjk2IGltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzcy1h bmltYXRpb25zL2tleWZyYW1lcy1yZW1vdmUtZG9jdW1lbnRFbGVtZW50LWNyYXNoLmh0bWwgWyBT a2lwIF0KLXdlYmtpdC5vcmcvYi8yMDM0ODAgaW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0 cy9jc3MvY3NzLWFuaW1hdGlvbnMvZXZlbnQtb3JkZXIudGVudGF0aXZlLmh0bWwgWyBQYXNzIEZh aWx1cmUgXQogd2Via2l0Lm9yZy9iLzIwMzMwNSBpbXBvcnRlZC93M2Mvd2ViLXBsYXRmb3JtLXRl c3RzL2Nzcy9jc3MtdHJhbnNpdGlvbnMvcHJvcGVydGllcy12YWx1ZS0wMDEuaHRtbCBbIFBhc3Mg RmFpbHVyZSBdCiB3ZWJraXQub3JnL2IvMjAzMzA1IGltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0t dGVzdHMvY3NzL2Nzcy10cmFuc2l0aW9ucy9wcm9wZXJ0aWVzLXZhbHVlLWluaGVyaXQtMDAxLmh0 bWwgWyBQYXNzIEZhaWx1cmUgXQogd2Via2l0Lm9yZy9iLzIwMzMwNSBpbXBvcnRlZC93M2Mvd2Vi LXBsYXRmb3JtLXRlc3RzL2Nzcy9jc3MtdHJhbnNpdGlvbnMvcHJvcGVydGllcy12YWx1ZS1pbmhl cml0LTAwMi5odG1sIFsgUGFzcyBGYWlsdXJlIF0K