198191 2019-05-23 12:11:03 -0700 [Pointer Events] Check that capturing data managed by the PointerCaptureController gets cleared upon navigation 2019-06-03 02:30:29 -0700 1 1 1 Unclassified WebKit UI Events WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=198129 InRadar P2 Normal --- 1 graouts graouts ap cdumez commit-queue darin dino rniwa webkit-bug-importer oldest_to_newest 1538740 0 graouts 2019-05-23 12:11:03 -0700 We capture some pointer state in the PointerCaptureController, and on macOS where the pointer id is always the same, we should check that the state is correctly reset upon navigation in case the user is in the middle of an interaction. This came up in the post-commit discussion of https://bugs.webkit.org/show_bug.cgi?id=198129. 1538742 1 webkit-bug-importer 2019-05-23 12:11:22 -0700 <rdar://problem/51077486> 1539365 2 370685 graouts 2019-05-27 03:05:04 -0700 Created attachment 370685 Patch 1539435 3 370685 ap 2019-05-27 13:11:45 -0700 Comment on attachment 370685 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370685&action=review > Source/WebCore/page/Page.cpp:2864 > + m_pointerCaptureController->reset(); What about subframe navigation? I don't think that we want these to leak between documents in frames either. 1539534 4 graouts 2019-05-28 05:23:52 -0700 (In reply to Alexey Proskuryakov from comment #3) > Comment on attachment 370685 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=370685&action=review > > > Source/WebCore/page/Page.cpp:2864 > > + m_pointerCaptureController->reset(); > > What about subframe navigation? I don't think that we want these to leak > between documents in frames either. I don't think there is anything to do in this case. The problem was that if you called preventDefault() while handling a "pointerdown" event in a page and navigated to the next page without releasing the mouse button, then compatibility mouse events would be prevented in the new page until you released the mouse button. There is one PointerCaptureController per Page, so the main document and sub-frame documents use the same one. If preventDefault() is called while handling a "pointerdown" event in a sub-frame document, even if that document is navigated, or the whole iframe removed, it sounds fine to me that the state set by the sub-frame document remains valid for the main frame or any other sub-frame. So I think this patch is correct and all that matters is clearing the state when the Page's main frame's document changes. 1539535 5 graouts 2019-05-28 05:33:23 -0700 Committed r245809: <https://trac.webkit.org/changeset/245809> 1539601 6 ap 2019-05-28 09:12:44 -0700 > There is one PointerCaptureController per Page, so the main document and > sub-frame documents use the same one. If preventDefault() is called while > handling a "pointerdown" event in a sub-frame document, even if that > document is navigated, or the whole iframe removed, it sounds fine to me > that the state set by the sub-frame document remains valid for the main > frame or any other sub-frame. Generally, the reasons why no event state should be preserved over page loads even for subframes are user visible behavior and security. Quickly changing documents in either main frame or subframe results in events being delivered to a wrong document, resulting in unexpected actions getting triggered by them. This is a variation of clickjacking. I don't see anything critical in this particular case, but this is the kind of behavior that we tend to see as incorrect. I other words, I guess that what I'm saying is that it's not right to have a single PointerCaptureController per page, all event dispatch state should be per document. 1539606 7 graouts 2019-05-28 09:21:26 -0700 (In reply to Alexey Proskuryakov from comment #6) > > I other words, I guess that what I'm saying is that it's not right to have a > single PointerCaptureController per page, all event dispatch state should be > per document. I thought about having a PointerCaptureController per Document, however I'm not sure how it would work in practice. A pointer travels across the main frame's content and all of its subframes, and as it does that it produces events across all frames. The behavior of the pointer and the event it produces requires coordination across all frames, and as such it makes most sense to me to have a single one per Page. Of course, we could have one per Document and introduce coordination between them, but it would definitely be a lot more work and the benefits are not obvious to me. 1539826 8 370685 darin 2019-05-28 17:27:01 -0700 Comment on attachment 370685 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370685&action=review > Source/WebCore/page/PointerCaptureController.cpp:151 > + m_activePointerIdsToCapturingData.set(mousePointerID, capturingData); Should use add instead of set; set includes unnecessary code to handle the case where the key already exists in the hash table. Impossible in a freshly cleared hash table. 1541202 9 graouts 2019-06-03 02:29:51 -0700 Committed r246031: <https://trac.webkit.org/changeset/246031> 1541203 10 graouts 2019-06-03 02:30:29 -0700 (In reply to Darin Adler from comment #8) > Comment on attachment 370685 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=370685&action=review > > > Source/WebCore/page/PointerCaptureController.cpp:151 > > + m_activePointerIdsToCapturingData.set(mousePointerID, capturingData); > > Should use add instead of set; set includes unnecessary code to handle the > case where the key already exists in the hash table. Impossible in a freshly > cleared hash table. Thanks Darin, this was addressed in the additional commit r246031. 370685 2019-05-27 03:05:04 -0700 2019-05-28 05:32:15 -0700 Patch bug-198191-20190527120502.patch text/plain 3945 graouts U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ1Nzc0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjAwNzg4MGNmNjQ5MzA2 OTZiZjVjODdhYTE1N2JiMTFiM2YzNjJjYS4uMTU1MmQzMzRiYzgwMGVhYTM1MWM2ZjlhNTUyODY2 ZjJhNDZkYjUzNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDE5LTA1LTI3ICBBbnRv aW5lIFF1aW50ICA8Z3Jhb3V0c0BhcHBsZS5jb20+CisKKyAgICAgICAgW1BvaW50ZXIgRXZlbnRz XSBDaGVjayB0aGF0IGNhcHR1cmluZyBkYXRhIG1hbmFnZWQgYnkgdGhlIFBvaW50ZXJDYXB0dXJl Q29udHJvbGxlciBnZXRzIGNsZWFyZWQgdXBvbiBuYXZpZ2F0aW9uCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTgxOTEKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXaGVuIHRoZSBkb2N1bWVudCBvZiB0aGUg cGFnZSdzIG1haW4gZnJhbWUgY2hhbmdlcywgbWFrZSBzdXJlIHdlIGNsZWFyIGFsbCBvZiB0aGUg ZGF0YSBhY2N1bXVsYXRlZCBmb3IgdGhlIHByZXZpb3VzIGRvY3VtZW50LgorICAgICAgICBJIGRv bid0IHRoaW5rIHRoaXMgcGFydGljdWxhciBjaGFuZ2UgaXMgdGVzdGFibGUgYXMgbm9uZSBvZiB0 aGUgZGF0YSBjb250YWluZWQgaW4gdGhlIFBvaW50ZXJJZFRvQ2FwdHVyaW5nRGF0YU1hcCBtYWlu dGFpbmVkIGJ5CisgICAgICAgIHRoZSBQb2ludGVyQ2FwdHVyZUNvbnRyb2xsZXIgY29udGFpbnMg YW55IGRhdGEgdGhhdCBjb3VsZCBiZSBpbnNwZWN0ZWQgYnkgdGhlIHBhZ2UgZHVlIHRvIG90aGVy IGZpeGVzIGxhbmRlZCB0byBmaXggd2tiLnVnLzE5ODEyOSwKKyAgICAgICAgYnV0IEkndmUgY2hl Y2tlZCB0aGF0IHJlbW92aW5nIHRob3NlIGZpeGVzIGFuZCB1c2luZyB0aGlzIHBhdGNoIGNvcnJl Y3RseSBmaXhlcyB0aGF0IGJ1Zy4KKworICAgICAgICAqIHBhZ2UvUGFnZS5jcHA6CisgICAgICAg IChXZWJDb3JlOjpQYWdlOjpkaWRDaGFuZ2VNYWluRG9jdW1lbnQpOgorICAgICAgICAqIHBhZ2Uv UG9pbnRlckNhcHR1cmVDb250cm9sbGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlBvaW50ZXJD YXB0dXJlQ29udHJvbGxlcjo6UG9pbnRlckNhcHR1cmVDb250cm9sbGVyKToKKyAgICAgICAgKFdl YkNvcmU6OlBvaW50ZXJDYXB0dXJlQ29udHJvbGxlcjo6cmVzZXQpOgorICAgICAgICAqIHBhZ2Uv UG9pbnRlckNhcHR1cmVDb250cm9sbGVyLmg6CisKIDIwMTktMDUtMjUgIEFudG9pbmUgUXVpbnQg IDxncmFvdXRzQGFwcGxlLmNvbT4KIAogICAgICAgICBPcHQgbmF2ZXIuY29tIGludG8gc2ltdWxh dGVkIG1vdXNlIGV2ZW50cyBxdWlyayBvbiBpT1MKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L3BhZ2UvUGFnZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wYWdlL1BhZ2UuY3BwCmluZGV4IGFhMDEz YmY3NjMzZWNlMDM2ZGU5MGJmZTc0NWJhNDM3ZGYyNTgwNWQuLjgxNjgxMTU3MjQ3MThmMTMzZWIw NjhkZWM5YmIwYzVhYTc3YjY2MTEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvUGFn ZS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGFnZS9QYWdlLmNwcApAQCAtMjg2MCw2ICsyODYw LDkgQEAgdm9pZCBQYWdlOjpkaWRDaGFuZ2VNYWluRG9jdW1lbnQoKQogI2lmIEVOQUJMRShXRUJf UlRDKQogICAgIG1fcnRjQ29udHJvbGxlci5yZXNldChtX3Nob3VsZEVuYWJsZUlDRUNhbmRpZGF0 ZUZpbHRlcmluZ0J5RGVmYXVsdCk7CiAjZW5kaWYKKyNpZiBFTkFCTEUoUE9JTlRFUl9FVkVOVFMp CisgICAgbV9wb2ludGVyQ2FwdHVyZUNvbnRyb2xsZXItPnJlc2V0KCk7CisjZW5kaWYKIH0KIAog UmVuZGVyaW5nVXBkYXRlU2NoZWR1bGVyJiBQYWdlOjpyZW5kZXJpbmdVcGRhdGVTY2hlZHVsZXIo KQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGFnZS9Qb2ludGVyQ2FwdHVyZUNvbnRyb2xs ZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGFnZS9Qb2ludGVyQ2FwdHVyZUNvbnRyb2xsZXIuY3Bw CmluZGV4IDk0MjIxNTIyZDFlYzQwNzZmOTBkMjMwYTI4YTZmNzEzZDc2OGJjY2EuLmVmYTEyYTE1 ZDliZDdlMTlkMmFlMTg0MTE2MGE4Njk0YTBhNjYzOTEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL3BhZ2UvUG9pbnRlckNhcHR1cmVDb250cm9sbGVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29y ZS9wYWdlL1BvaW50ZXJDYXB0dXJlQ29udHJvbGxlci5jcHAKQEAgLTQ0LDExICs0NCw3IEBAIG5h bWVzcGFjZSBXZWJDb3JlIHsKIFBvaW50ZXJDYXB0dXJlQ29udHJvbGxlcjo6UG9pbnRlckNhcHR1 cmVDb250cm9sbGVyKFBhZ2UmIHBhZ2UpCiAgICAgOiBtX3BhZ2UocGFnZSkKIHsKLSNpZiAhRU5B QkxFKFRPVUNIX0VWRU5UUykKLSAgICBDYXB0dXJpbmdEYXRhIGNhcHR1cmluZ0RhdGE7Ci0gICAg Y2FwdHVyaW5nRGF0YS5wb2ludGVyVHlwZSA9IFBvaW50ZXJFdmVudDo6bW91c2VQb2ludGVyVHlw ZSgpOwotICAgIG1fYWN0aXZlUG9pbnRlcklkc1RvQ2FwdHVyaW5nRGF0YS5zZXQobW91c2VQb2lu dGVySUQsIGNhcHR1cmluZ0RhdGEpOwotI2VuZGlmCisgICAgcmVzZXQoKTsKIH0KIAogRXhjZXB0 aW9uT3I8dm9pZD4gUG9pbnRlckNhcHR1cmVDb250cm9sbGVyOjpzZXRQb2ludGVyQ2FwdHVyZShF bGVtZW50KiBjYXB0dXJpbmdUYXJnZXQsIFBvaW50ZXJJRCBwb2ludGVySWQpCkBAIC0xNDYsNiAr MTQyLDE2IEBAIHZvaWQgUG9pbnRlckNhcHR1cmVDb250cm9sbGVyOjplbGVtZW50V2FzUmVtb3Zl ZChFbGVtZW50JiBlbGVtZW50KQogICAgIH0KIH0KIAordm9pZCBQb2ludGVyQ2FwdHVyZUNvbnRy b2xsZXI6OnJlc2V0KCkKK3sKKyAgICBtX2FjdGl2ZVBvaW50ZXJJZHNUb0NhcHR1cmluZ0RhdGEu Y2xlYXIoKTsKKyNpZiAhRU5BQkxFKFRPVUNIX0VWRU5UUykKKyAgICBDYXB0dXJpbmdEYXRhIGNh cHR1cmluZ0RhdGE7CisgICAgY2FwdHVyaW5nRGF0YS5wb2ludGVyVHlwZSA9IFBvaW50ZXJFdmVu dDo6bW91c2VQb2ludGVyVHlwZSgpOworICAgIG1fYWN0aXZlUG9pbnRlcklkc1RvQ2FwdHVyaW5n RGF0YS5zZXQobW91c2VQb2ludGVySUQsIGNhcHR1cmluZ0RhdGEpOworI2VuZGlmCit9CisKIHZv aWQgUG9pbnRlckNhcHR1cmVDb250cm9sbGVyOjp0b3VjaFdpdGhJZGVudGlmaWVyV2FzUmVtb3Zl ZChQb2ludGVySUQgcG9pbnRlcklkKQogewogICAgIG1fYWN0aXZlUG9pbnRlcklkc1RvQ2FwdHVy aW5nRGF0YS5yZW1vdmUocG9pbnRlcklkKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3Bh Z2UvUG9pbnRlckNhcHR1cmVDb250cm9sbGVyLmggYi9Tb3VyY2UvV2ViQ29yZS9wYWdlL1BvaW50 ZXJDYXB0dXJlQ29udHJvbGxlci5oCmluZGV4IDBhMjdjNzlkNmQ2ODljYzk3NWZjNjIwNDMyMDk3 Y2IyMjc0N2JiZDUuLjQzMTFjNDZlMTNkMjA2NzhkODM2MWU3YTA2ZmVhMzkwNGJiMTlmYzMgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvUG9pbnRlckNhcHR1cmVDb250cm9sbGVyLmgK KysrIGIvU291cmNlL1dlYkNvcmUvcGFnZS9Qb2ludGVyQ2FwdHVyZUNvbnRyb2xsZXIuaApAQCAt NDQsNiArNDQsNyBAQCBwdWJsaWM6CiAgICAgRXhjZXB0aW9uT3I8dm9pZD4gc2V0UG9pbnRlckNh cHR1cmUoRWxlbWVudCosIFBvaW50ZXJJRCk7CiAgICAgRXhjZXB0aW9uT3I8dm9pZD4gcmVsZWFz ZVBvaW50ZXJDYXB0dXJlKEVsZW1lbnQqLCBQb2ludGVySUQpOwogICAgIGJvb2wgaGFzUG9pbnRl ckNhcHR1cmUoRWxlbWVudCosIFBvaW50ZXJJRCk7CisgICAgdm9pZCByZXNldCgpOwogCiAgICAg dm9pZCBwb2ludGVyTG9ja1dhc0FwcGxpZWQoKTsKICAgICB2b2lkIGVsZW1lbnRXYXNSZW1vdmVk KEVsZW1lbnQmKTsK