191694 2018-11-15 09:06:13 -0800 Click on node assigned to slot in button's shadow cause loss of button focus 2018-12-03 12:57:24 -0800 1 1 1 Unclassified WebKit DOM Safari 12 Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=191851 InRadar P2 Normal --- 148695 1 jan rniwa cdumez jan koivisto rniwa simon.fraser webkit-bug-importer wenson_hsieh oldest_to_newest 1478932 0 jan 2018-11-15 09:06:13 -0800 See discussion at: https://github.com/w3c/webcomponents/issues/773 Summary: Consider a custom element whose shadow contains a focusable element that contains a slot: <test-element> #shadow-root <div tabindex="0"> <slot></slot> </div> </test-element> And suppose this custom element receives some light DOM children: <test-element> <span>Click me</span> </test-element> Expect: a click on the light DOM span should give focus to the focusable div inside test-element. Actual: the focus goes to the document body. Repro: https://jsbin.com/qivoheg/edit?html,output. Firefox exhibits the desired behavior here. Chrome currently has a bug to fix this, rniwa indicated in the linked discussion that this should be fixed in WebKit too. 1479114 1 rniwa 2018-11-15 14:19:01 -0800 *** Bug 111231 has been marked as a duplicate of this bug. *** 1479116 2 webkit-bug-importer 2018-11-15 14:19:45 -0800 <rdar://problem/46107920> 1480366 3 355308 rniwa 2018-11-19 21:16:31 -0800 Created attachment 355308 Fixes the bug 1480373 4 355308 wenson_hsieh 2018-11-19 21:44:26 -0800 Comment on attachment 355308 Fixes the bug r=mews As an aside, it looks like there's similar logic in FrameSelection::setFocusedElementIfNeeded() that attempts to walk up the DOM in search of an element to focus. 1480375 5 rniwa 2018-11-19 21:47:28 -0800 (In reply to Wenson Hsieh from comment #4) > Comment on attachment 355308 [details] > Fixes the bug > > r=mews > > As an aside, it looks like there's similar logic in > FrameSelection::setFocusedElementIfNeeded() that attempts to walk up the DOM > in search of an element to focus. Since we don't really allow the edibility to propagate across shadow boundaries, that one might be okay. It's probably also never going t one useful in real life. 1480414 6 rniwa 2018-11-20 00:53:59 -0800 Committed r238393: <https://trac.webkit.org/changeset/238393> 1480447 7 355308 koivisto 2018-11-20 03:55:39 -0800 Comment on attachment 355308 Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=355308&action=review > Source/WebCore/page/EventHandler.cpp:2602 > // Walk up the DOM tree to search for an element to focus. > - Element* element; > - for (element = m_elementUnderMouse.get(); element; element = element->parentOrShadowHostElement()) { > + RefPtr<Element> element; > + for (element = m_elementUnderMouse.get(); element; element = element->parentElementInComposedTree()) { Most (all?) clients of parentOrShadowHostElement() are buggy or work correctly by accident (the context can't have slots). It would be good to get rid of it entirely. 1480619 8 rniwa 2018-11-20 16:17:08 -0800 (In reply to Antti Koivisto from comment #7) > Comment on attachment 355308 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=355308&action=review > > > Source/WebCore/page/EventHandler.cpp:2602 > > // Walk up the DOM tree to search for an element to focus. > > - Element* element; > > - for (element = m_elementUnderMouse.get(); element; element = element->parentOrShadowHostElement()) { > > + RefPtr<Element> element; > > + for (element = m_elementUnderMouse.get(); element; element = element->parentElementInComposedTree()) { > > Most (all?) clients of parentOrShadowHostElement() are buggy or work > correctly by accident (the context can't have slots). It would be good to > get rid of it entirely. Yeah, but I think it requires quite a bit of figuring out what makes sense in each case. e.g. Element::isSpellCheckingEnabled uses parentOrShadowHostElement but it's non-sensical to call parentElementInComposedTree there since the content editable doesn't propagate across shadow boundaries. parentNode() probably makes more sense there but perhaps we'd have to special case input/textarea because when spellcheck=false is set on input/textarea, we're supposed to disable spellchecking. 1484456 9 355308 rniwa 2018-12-03 12:57:24 -0800 Comment on attachment 355308 Fixes the bug The patch had already been landed. 355308 2018-11-19 21:16:31 -0800 2018-12-03 12:57:24 -0800 Fixes the bug fix191694.patch text/plain 4946 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDIzODM4OSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDE4LTExLTE5ICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIENsaWNrIG9uIG5vZGUgYXNzaWdu ZWQgdG8gc2xvdCBpbiBidXR0b24ncyBzaGFkb3cgY2F1c2UgbG9zcyBvZiBidXR0b24gZm9jdXMK KyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE5MTY5NAor ICAgICAgICA8cmRhcjovL3Byb2JsZW0vNDYxMDc5MjA+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4ZWQgdGhlIGJ1ZyBieSB0cmF2ZXJzaW5nIHRo ZSBwYXJlbnQgaW4gdGhlIGNvbXBvc2VkIHRyZWUgd2hlbiBsb29raW5nIGZvciBhbiBlbGVtZW50 IHRvIGZvY3VzLgorCisgICAgICAgIFRlc3Q6IGZhc3Qvc2hhZG93LWRvbS9mb2N1cy1zbG90LXBh cmVudC5odG1sCisKKyAgICAgICAgKiBwYWdlL0V2ZW50SGFuZGxlci5jcHA6CisgICAgICAgIChX ZWJDb3JlOjpFdmVudEhhbmRsZXI6OmRpc3BhdGNoTW91c2VFdmVudCk6IEZpeGVkIHRoZSBidWcu IEFsc28gdXNlIFJlZlB0ciBpbnN0ZWFkIG9mIGEgcmF3IHBvaW50ZXIuCisKIDIwMTgtMTEtMTkg IEJhc3VrZSBTdXp1a2kgIDxiYXN1a2Uuc3V6dWtpQHNvbnkuY29tPgogCiAgICAgICAgIFtDdXJs XSBBZGQgQVBJIGZvciBDZXJ0aWZpY2F0ZUluZm8uCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wYWdl L0V2ZW50SGFuZGxlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGFnZS9FdmVu dEhhbmRsZXIuY3BwCShyZXZpc2lvbiAyMzgzODIpCisrKyBTb3VyY2UvV2ViQ29yZS9wYWdlL0V2 ZW50SGFuZGxlci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTI1OTgsOCArMjU5OCw4IEBAIGJvb2wg RXZlbnRIYW5kbGVyOjpkaXNwYXRjaE1vdXNlRXZlbnQoY28KICAgICAvLyBmcm9tIHRoZSB1c2Vy IGludGVyZmFjZSBvZiBXaW5kb3dzLCB3aGVyZSBwdXNoaW5nIGEgYnV0dG9uIG1vdmVzIGZvY3Vz IHRvIHRoZSBidXR0b24uCiAKICAgICAvLyBXYWxrIHVwIHRoZSBET00gdHJlZSB0byBzZWFyY2gg Zm9yIGFuIGVsZW1lbnQgdG8gZm9jdXMuCi0gICAgRWxlbWVudCogZWxlbWVudDsKLSAgICBmb3Ig KGVsZW1lbnQgPSBtX2VsZW1lbnRVbmRlck1vdXNlLmdldCgpOyBlbGVtZW50OyBlbGVtZW50ID0g ZWxlbWVudC0+cGFyZW50T3JTaGFkb3dIb3N0RWxlbWVudCgpKSB7CisgICAgUmVmUHRyPEVsZW1l bnQ+IGVsZW1lbnQ7CisgICAgZm9yIChlbGVtZW50ID0gbV9lbGVtZW50VW5kZXJNb3VzZS5nZXQo KTsgZWxlbWVudDsgZWxlbWVudCA9IGVsZW1lbnQtPnBhcmVudEVsZW1lbnRJbkNvbXBvc2VkVHJl ZSgpKSB7CiAgICAgICAgIGlmIChlbGVtZW50LT5pc01vdXNlRm9jdXNhYmxlKCkpCiAgICAgICAg ICAgICBicmVhazsKICAgICB9CkBAIC0yNjIyLDcgKzI2MjIsNyBAQCBib29sIEV2ZW50SGFuZGxl cjo6ZGlzcGF0Y2hNb3VzZUV2ZW50KGNvCiAKICAgICAvLyBJZiBmb2N1cyBzaGlmdCBpcyBibG9j a2VkLCB3ZSBlYXQgdGhlIGV2ZW50LgogICAgIGF1dG8qIHBhZ2UgPSBtX2ZyYW1lLnBhZ2UoKTsK LSAgICBpZiAocGFnZSAmJiAhcGFnZS0+Zm9jdXNDb250cm9sbGVyKCkuc2V0Rm9jdXNlZEVsZW1l bnQoZWxlbWVudCwgbV9mcmFtZSkpCisgICAgaWYgKHBhZ2UgJiYgIXBhZ2UtPmZvY3VzQ29udHJv bGxlcigpLnNldEZvY3VzZWRFbGVtZW50KGVsZW1lbnQuZ2V0KCksIG1fZnJhbWUpKQogICAgICAg ICByZXR1cm4gZmFsc2U7CiAKICAgICByZXR1cm4gdHJ1ZTsKSW5kZXg6IExheW91dFRlc3RzL0No YW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDIz ODM4MikKKysrIExheW91dFRlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsx LDE2IEBACisyMDE4LTExLTE5ICBSeW9zdWtlIE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisg ICAgICAgIENsaWNrIG9uIG5vZGUgYXNzaWduZWQgdG8gc2xvdCBpbiBidXR0b24ncyBzaGFkb3cg Y2F1c2UgbG9zcyBvZiBidXR0b24gZm9jdXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTE5MTY5NAorICAgICAgICA8cmRhcjovL3Byb2JsZW0vNDYxMDc5 MjA+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQWRk ZWQgYSByZWdyZXNzaW9uIHRlc3QuCisKKyAgICAgICAgKiBmYXN0L3NoYWRvdy1kb20vZm9jdXMt c2xvdC1wYXJlbnQtZXhwZWN0ZWQudHh0OiBBZGRlZC4KKyAgICAgICAgKiBmYXN0L3NoYWRvdy1k b20vZm9jdXMtc2xvdC1wYXJlbnQuaHRtbDogQWRkZWQuCisKIDIwMTgtMTEtMTkgIEp1c3RpbiBG YW4gIDxqdXN0aW5fZmFuQGFwcGxlLmNvbT4KIAogICAgICAgICBbV2ViR1BVXSBCZWdpbiBpbXBs ZW1lbnRpbmcgV2ViR1BVVGV4dHVyZSwgV2ViR1BVVGV4dHVyZVZpZXcsIGFuZCBXZWJHUFVUZXh0 dXJlRm9ybWF0RW51bSwgYW5kIFdlYkdQVVN3YXBDaGFpbjo6Y29uZmlndXJlIHVwZ3JhZGVzCklu ZGV4OiBMYXlvdXRUZXN0cy9mYXN0L3NoYWRvdy1kb20vZm9jdXMtc2xvdC1wYXJlbnQtZXhwZWN0 ZWQudHh0Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL2Zhc3Qvc2hhZG93LWRvbS9mb2N1cy1z bG90LXBhcmVudC1leHBlY3RlZC50eHQJKG5vbmV4aXN0ZW50KQorKysgTGF5b3V0VGVzdHMvZmFz dC9zaGFkb3ctZG9tL2ZvY3VzLXNsb3QtcGFyZW50LWV4cGVjdGVkLnR4dAkod29ya2luZyBjb3B5 KQpAQCAtMCwwICsxLDExIEBACitUaGlzIHRlc3RzIGNsaWNraW5nIG9uIGEgc2xvdHRlZCBjb250 ZW50IHdvdWxkIGZvY3VzIHRoZSBhbmNlc3RvciBvZiB0aGUgc2xvdCBlbGVtZW50IGl0IGJlbG9u Z3MuCitUbyBtYW5hdWxseSB0ZXN0LCBjbGljayBvbiAiQ2xpY2sgaGVyZSIgYmVsb3cuCisKK09u IHN1Y2Nlc3MsIHlvdSB3aWxsIHNlZSBhIHNlcmllcyBvZiAiUEFTUyIgbWVzc2FnZXMsIGZvbGxv d2VkIGJ5ICJURVNUIENPTVBMRVRFIi4KKworCitQQVNTIGZvY3VzRXZlbnRUYXJnZXQgaXMgZm9j dXNhYmxlU2xvdFBhcmVudAorUEFTUyBzdWNjZXNzZnVsbHlQYXJzZWQgaXMgdHJ1ZQorCitURVNU IENPTVBMRVRFCisKSW5kZXg6IExheW91dFRlc3RzL2Zhc3Qvc2hhZG93LWRvbS9mb2N1cy1zbG90 LXBhcmVudC5odG1sCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL2Zhc3Qvc2hhZG93LWRvbS9m b2N1cy1zbG90LXBhcmVudC5odG1sCShub25leGlzdGVudCkKKysrIExheW91dFRlc3RzL2Zhc3Qv c2hhZG93LWRvbS9mb2N1cy1zbG90LXBhcmVudC5odG1sCSh3b3JraW5nIGNvcHkpCkBAIC0wLDAg KzEsMzggQEAKKzwhRE9DVFlQRSBodG1sPgorPGh0bWw+Cis8Ym9keT4KKzxkaXYgaWQ9Imhvc3Qi PjxzcGFuIG9uY2xpY2s9ImNoZWNrRm9jdXNGaXJlZCgpIj5DbGljayBoZXJlPC9zcGFuPjwvZGl2 PgorPHNjcmlwdCBzcmM9Ii4uLy4uL3Jlc291cmNlcy9qcy10ZXN0LmpzIj48L3NjcmlwdD4KKzxz Y3JpcHQgc3JjPSIuLi8uLi9yZXNvdXJjZXMvdWktaGVscGVyLmpzIj48L3NjcmlwdD4KKzxzY3Jp cHQ+CisKK2Rlc2NyaXB0aW9uKGBUaGlzIHRlc3RzIGNsaWNraW5nIG9uIGEgc2xvdHRlZCBjb250 ZW50IHdvdWxkIGZvY3VzIHRoZSBhbmNlc3RvciBvZiB0aGUgc2xvdCBlbGVtZW50IGl0IGJlbG9u Z3MuPGJyPgorVG8gbWFuYXVsbHkgdGVzdCwgY2xpY2sgb24gIkNsaWNrIGhlcmUiIGJlbG93LmAp OworCitjb25zdCBzaGFkb3dSb290ID0gaG9zdC5hdHRhY2hTaGFkb3coe21vZGU6ICdjbG9zZWQn fSk7CitzaGFkb3dSb290LmlubmVySFRNTCA9ICc8c3BhbiB0YWJpbmRleD0iMCI+PHNsb3Q+PC9z bG90Pjwvc3Bhbj4nOworCit3aW5kb3cuZm9jdXNhYmxlU2xvdFBhcmVudCA9IHNoYWRvd1Jvb3Qu cXVlcnlTZWxlY3RvcignW3RhYmluZGV4XScpOword2luZG93LmZvY3VzRXZlbnRUYXJnZXQgPSBu dWxsOworZm9jdXNhYmxlU2xvdFBhcmVudC5hZGRFdmVudExpc3RlbmVyKCdmb2N1cycsIChldmVu dCkgPT4geworICAgIHdpbmRvdy5mb2N1c0V2ZW50VGFyZ2V0ID0gZXZlbnQudGFyZ2V0OworfSk7 CisKK2Z1bmN0aW9uIGNoZWNrRm9jdXNGaXJlZCgpCit7CisgICAgc2hvdWxkQmUoJ2ZvY3VzRXZl bnRUYXJnZXQnLCAnZm9jdXNhYmxlU2xvdFBhcmVudCcpOworICAgIGhvc3QucmVtb3ZlKCk7Cisg ICAgZmluaXNoSlNUZXN0KCk7Cit9CisKK2lmICh3aW5kb3cudGVzdFJ1bm5lciAmJiAhd2luZG93 LmV2ZW50U2VuZGVyKQorICAgIGRvY3VtZW50LndyaXRlKCdUaGlzIHRlc3QgcmVxdWlyZXMgZXZl bnRTZW5kZXInKTsKK2Vsc2UgeworICAgIGlmICh3aW5kb3cuZXZlbnRTZW5kZXIpCisgICAgICAg IHdpbmRvdy5vbmxvYWQgPSBhc3luYyAoKSA9PiBhd2FpdCBVSUhlbHBlci5hY3RpdmF0ZUF0KGhv c3Qub2Zmc2V0TGVmdCArIDUsIGhvc3Qub2Zmc2V0VG9wICsgNSk7CisgICAganNUZXN0SXNBc3lu YyA9IHRydWU7Cit9CisKKzwvc2NyaXB0PgorPC9ib2R5PgorPC9odG1sPgo=