82593 2012-03-29 03:40:25 -0700 Assertion fails when a focus navigation traverse a shadow host whose shadow root does not have any children. 2012-04-04 19:07:48 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 78585 1 hayato hayato dglazkov dominicc morrita rolandsteiner shinyak webkit.review.bot oldest_to_newest 591014 0 hayato 2012-03-29 03:40:25 -0700 I'll upload a test case. This should not happen on release build.This happens only on debug build. Let me explain the detail later. We should revise current traversal APIs so that they can handle the case when ShadowRoot is passed directly as a parameter. 591018 1 134542 hayato 2012-03-29 03:54:37 -0700 Created attachment 134542 crash case 591028 2 hayato 2012-03-29 04:19:23 -0700 Let me summarize what I am thinking now: - Most of public APIs defined in ReifiedTreeTraversal is assuming that it does not take ShadowRoot as a parameter. I'd like to keep this rule since I don't want to break the following basic assumption: node.firstChild.parent == node (Please consider all traversing operation are done by ReifiedTreeTraversal APIs. I am using concise notation for laziness) If we allow some APIs can take ShadowRoot as parameter, this basic rule will be broken: shadowRoot.firstChild.parent == shadowHost of shadowRoot != shadowRoot But there are some use cases that we'd like to traverse a node, using ShadowRoot as a start node, like a FocusController. So I think we should change the behaviors as follows - If crossing upper boundary is allowed: - APIs must not take ShadowRoot as a parameter. - APIs won't return ShadowRoot as a return value. - If crossing upper boundary is disallowed: - APIs can take ShadowRoot as a parameter. -Some APIs (parentNode) may return youngest ShadowRoot as a return value. I think this can solve the issue. I'd like to implement this on Walker APIs, which will be a successor of current ReifiedTreeTraversal APIs. 591238 3 dglazkov 2012-03-29 09:15:35 -0700 (In reply to comment #2) > Let me summarize what I am thinking now: > > - Most of public APIs defined in ReifiedTreeTraversal is assuming that it does not take ShadowRoot as a parameter. I'd like to keep this rule since I don't want to break the following basic assumption: > > node.firstChild.parent == node > > (Please consider all traversing operation are done by ReifiedTreeTraversal APIs. I am using concise notation for laziness) > > If we allow some APIs can take ShadowRoot as parameter, this basic rule will be broken: > > shadowRoot.firstChild.parent == shadowHost of shadowRoot != shadowRoot > > But there are some use cases that we'd like to traverse a node, using ShadowRoot as a start node, like a FocusController. > So I think we should change the behaviors as follows > > - If crossing upper boundary is allowed: > - APIs must not take ShadowRoot as a parameter. > - APIs won't return ShadowRoot as a return value. > > - If crossing upper boundary is disallowed: > - APIs can take ShadowRoot as a parameter. > -Some APIs (parentNode) may return youngest ShadowRoot as a return value. I like it. This made me think some more about the walker API. What if the walker constructor took parameter(s) that define how traversal must happen? For example, ComposedShadowTreeWalker(node, AllowCrossingUpperBoundary, ...). Then, the walker functions could mirror Node traversal API. Will this work? WDYT? > I think this can solve the issue. > I'd like to implement this on Walker APIs, which will be a successor of current ReifiedTreeTraversal APIs. 591927 4 hayato 2012-03-30 00:26:06 -0700 (In reply to comment #3) > I like it. This made me think some more about the walker API. What if the walker constructor took parameter(s) that define how traversal must happen? For example, ComposedShadowTreeWalker(node, AllowCrossingUpperBoundary, ...). Then, the walker functions could mirror Node traversal API. Will this work? WDYT? That's the exactly same approach I had been taken on Walker. I am glad to hear that you have the same opinion by coincidence :). https://bugs.webkit.org/show_bug.cgi?id=82009 will be ready for a review in a few hours. 595268 5 135518 hayato 2012-04-03 23:16:35 -0700 Created attachment 135518 update a test 596220 6 135518 webkit.review.bot 2012-04-04 19:07:44 -0700 Comment on attachment 135518 update a test Clearing flags on attachment: 135518 Committed r113275: <http://trac.webkit.org/changeset/113275> 596221 7 webkit.review.bot 2012-04-04 19:07:48 -0700 All reviewed patches have been landed. Closing bug. 134542 2012-03-29 03:54:37 -0700 2012-04-03 23:16:29 -0700 crash case bug-82593-20120329195436.patch text/plain 2442 hayato U3VidmVyc2lvbiBSZXZpc2lvbjogMTEyNTE3CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggMzBmNzc0ZmJiMjkyNDhmZmFhODAw MGZjNDMyMDNlMTZiZWQwNDBkYi4uNGE2ZmI1N2I5ZjljOWNmNjc2MDlmYTU4YmM4NjhlZDI5ODgx NmIxMCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDEyIEBACisyMDEyLTAzLTI5ICBIYXlhdG8gSXRvICA8aGF5 YXRvQGNocm9taXVtLm9yZz4KKworICAgICAgICBBc3NlcnRpb24gZmFpbHMgd2hlbiBhIGZvY3Vz IG5hdmlnYXRpb24gdHJhdmVyc2UgYSBzaGFkb3cgaG9zdCB3aG9zZSBzaGFkb3cgcm9vdCBkb2Vz IG5vdCBoYXZlIGFueSBjaGlsZHJlbi4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTgyNTkzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgKiBmYXN0L2RvbS9zaGFkb3cvZm9jdXMtbmF2aWdhdGlvbi5odG1sOgor CiAyMDEyLTAzLTI5ICBaZW5vIEFsYmlzc2VyICA8emVub0B3ZWJraXQub3JnPgogCiAgICAgICAg IEZpZWxkc2V0IGRpc2FibGVkIGF0dHJpYnV0ZSBkb2VzIG5vdCB3b3JrLgpkaWZmIC0tZ2l0IGEv TGF5b3V0VGVzdHMvZmFzdC9kb20vc2hhZG93L2ZvY3VzLW5hdmlnYXRpb24uaHRtbCBiL0xheW91 dFRlc3RzL2Zhc3QvZG9tL3NoYWRvdy9mb2N1cy1uYXZpZ2F0aW9uLmh0bWwKaW5kZXggN2U3MDk1 NzBkZWJlYzMyZDY5OGFhNjRmMWEyNmQyN2U2ZGZiZjg3Ni4uYzdiMWUxOGFmNGI5MWRmNTkxNDIw OTNjZDBhODk3ZTgzYWExYTk5ZiAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvZmFzdC9kb20vc2hh ZG93L2ZvY3VzLW5hdmlnYXRpb24uaHRtbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2RvbS9zaGFk b3cvZm9jdXMtbmF2aWdhdGlvbi5odG1sCkBAIC01MCw4ICs1MCwxMSBAQCBmdW5jdGlvbiBwcmVw YXJlRE9NVHJlZShwYXJlbnQpCiAgICAgICAgICAgICAgICAgICBjcmVhdGVET00oJ2RpdicsIHsn aWQnOiAnaG9zdC1DJywgJ3RhYmluZGV4JzogLTF9LAogICAgICAgICAgICAgICAgICAgICAgICAg ICAgIGNyZWF0ZVNoYWRvd1Jvb3QoCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGNy ZWF0ZURPTSgnaW5wdXQnLCB7J3RhYmluZGV4JzogLTF9KSkpLAotICAgICAgICAgICAgICAgICAg Y3JlYXRlRE9NKCdpbnB1dCcsIHsnaWQnOiAnaW5wdXQtRC0wJywgJ3RhYmluZGV4JzogMH0pKSk7 Ci0KKyAgICAgICAgICAgICAgICAgIGNyZWF0ZURPTSgnaW5wdXQnLCB7J2lkJzogJ2lucHV0LUQt MCcsICd0YWJpbmRleCc6IDB9KSwKKyAgICAgICAgICAgICAgICAgIGNyZWF0ZURPTSgnZGl2Jywg e30sCisgICAgICAgICAgICAgICAgICAgICAgICAgICAgY3JlYXRlRE9NKCdkaXYnLCB7J2lkJzog J2hvc3QtRCcsICd0YWJpbmRleCc6IDB9LAorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICBjcmVhdGVTaGFkb3dSb290KCkpKSwKKyAgICAgICAgICAgICAgICAgIGNyZWF0ZURP TSgnaW5wdXQnLCB7J2lkJzogJ2lucHV0LUUtMCcsICd0YWJpbmRleCc6IDB9KSkpOwogICAgIHBh cmVudC5vZmZzZXRMZWZ0OwogfQogCkBAIC0xMTcsNiArMTIwLDggQEAgZnVuY3Rpb24gdGVzdCgp IHsKIAogICAgICAgICAvLyBBIG5vbi1mb2N1c2FibGUgc2hhZG93IGhvc3QgKGlkPWhvc3QtQyks IHdoaWNoIGRvZXMgbm90IGhhdmUgZm9jdXNhYmxlIGVsZW1lbnRzLCBzaG91bGQgYmUgc2tpcHBl ZCBlbnRpcmVseS4KICAgICAgICAgJ2lucHV0LUQtMCcsCisgICAgICAgICdob3N0LUQnLAorICAg ICAgICAnaW5wdXQtRS0wJywKIAogICAgICAgICAvLyBXcmFwcyB0byB0aGUgZmlyc3QgZWxlbWVu dCBpbiB0aGUgb3V0ZXJtb3N0IGZvY3VzIHNjb3BlLgogICAgICAgICAnaW5wdXQtQS0xJywKQEAg LTEyNSw2ICsxMzAsOCBAQCBmdW5jdGlvbiB0ZXN0KCkgewogCiAgICAgZWxlbWVudHNOYXZpZ2F0 ZWRJbkJhY2t3YXJkID0gWwogICAgICAgICAnaW5wdXQtQS0xJywKKyAgICAgICAgJ2lucHV0LUUt MCcsCisgICAgICAgICdob3N0LUQnLAogICAgICAgICAnaW5wdXQtRC0wJywKICAgICAgICAgJ2lu cHV0LUMtMCcsCiAgICAgICAgICdpbnB1dC1CLTAnLApAQCAtMTYzLDcgKzE3MCw3IEBAIGZ1bmN0 aW9uIHRlc3QoKSB7CiAgICAgICAgICdpbnB1dC1BLTEnLAogCiAgICAgICAgIC8vIFdyYXBzIHRv IHRoZSBsYXN0IGVsZW1lbnQgaW4gdGhlIG91dGVybW9zdCBmb2N1cyBzY29wZS4KLSAgICAgICAg J2lucHV0LUQtMCcsCisgICAgICAgICdpbnB1dC1FLTAnLAogICAgIF07CiAKICAgICB0ZXN0Rm9j dXNOYXZpZ2F0aW9uRm93cmFkKGVsZW1lbnRzTmF2aWdhdGVkSW5Gb3J3YXJkKTsK 135518 2012-04-03 23:16:35 -0700 2012-04-04 19:07:43 -0700 update a test bug-82593-20120404151633.patch text/plain 4080 hayato U3VidmVyc2lvbiBSZXZpc2lvbjogMTEzMTM0CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggMjRjNDIwODFkOWY1N2ZjOWFmODk3 ZDBiMzA0YmIxZjc3NmI5NGY2ZS4uMzI1NWRlZTViY2E2YjNmMDA3ZGY4MjlhMWE3MDYwOGEyNTkx ZDhhMSAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEyLTAzLTI5ICBIYXlhdG8gSXRvICA8aGF5 YXRvQGNocm9taXVtLm9yZz4KKworICAgICAgICBVcGRhdGUgYSBsYXlvdXQgdGVzdCwgYWRkaW5n IGEgY2FzZSBmb3IgdHJhdmVyc2luZyBhIFNoYWRvd1Jvb3Qgd2hpY2ggZG9lcyBub3QgaGF2ZSBh bnkgY2hpbGRyZW4uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD04MjU5MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFByaW9yIHRvIHIxMTMxMjUsIGlmIHdlIHRyYXZlcnNlIGEgU2hhZG93Um9vdCB3aGljaCBk b2VzIG5vdCBoYXZlIGFueSBjaGlsZHJlbiwKKyAgICAgICAgaXQgY2F1c2VkIGFuIGFzc2VydGlv biBmYWlsdXJlIG9uIGRlYnVnIGJ1aWxkLiByMTEzMTI1IGZpeGVkIHRoaXMgaXNzdWUgaW1wbGlj aXRseS4KKyAgICAgICAgQWRkcyBhIHRlc3QgY2FzZSBmb3IgdGhhdCB0byBjYXRjaCBhIGZ1dHVy ZSByZWdyZXNzaW9uLgorCisgICAgICAgICogZmFzdC9kb20vc2hhZG93L2ZvY3VzLW5hdmlnYXRp b24tZXhwZWN0ZWQuaHRtbDoKKyAgICAgICAgKiBmYXN0L2RvbS9zaGFkb3cvZm9jdXMtbmF2aWdh dGlvbi5odG1sOgorCiAyMDEyLTA0LTAzICBEaXJrIFByYW5rZSAgPGRwcmFua2VAY2hyb21pdW0u b3JnPgogCiAgICAgICAgIFVwZGF0ZSBhbm90aGVyIGJhc2VsaW5lIGZvciBhIGZhaWxpbmcgY2hy b21pdW0gU0wgc3ZnIHRlc3QgKGFuZCBtYXJrIGl0IGZsYWt5KS4KZGlmZiAtLWdpdCBhL0xheW91 dFRlc3RzL2Zhc3QvZG9tL3NoYWRvdy9mb2N1cy1uYXZpZ2F0aW9uLWV4cGVjdGVkLnR4dCBiL0xh eW91dFRlc3RzL2Zhc3QvZG9tL3NoYWRvdy9mb2N1cy1uYXZpZ2F0aW9uLWV4cGVjdGVkLnR4dApp bmRleCAwMTk1OGRlMzZlOTUwOTRlY2IzMGYwMWIzZmE0Y2YyOGI2ZGE0YzBkLi5iN2M2YzQxYThl MGMxNmM3Y2U2N2U2YzM4ZGY2YzFjYWFmMTdlYmM5IDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9m YXN0L2RvbS9zaGFkb3cvZm9jdXMtbmF2aWdhdGlvbi1leHBlY3RlZC50eHQKKysrIGIvTGF5b3V0 VGVzdHMvZmFzdC9kb20vc2hhZG93L2ZvY3VzLW5hdmlnYXRpb24tZXhwZWN0ZWQudHh0CkBAIC00 Niw5ICs0NiwxNyBAQCBTaG91bGQgbW92ZSBmcm9tIGlucHV0LUItMCB0byBpbnB1dC1DLTAgaW4g Zm9yd2FyZAogUEFTUwogU2hvdWxkIG1vdmUgZnJvbSBpbnB1dC1DLTAgdG8gaW5wdXQtRC0wIGlu IGZvcndhcmQKIFBBU1MKLVNob3VsZCBtb3ZlIGZyb20gaW5wdXQtRC0wIHRvIGlucHV0LUEtMSBp biBmb3J3YXJkCitTaG91bGQgbW92ZSBmcm9tIGlucHV0LUQtMCB0byBob3N0LUQgaW4gZm9yd2Fy ZAogUEFTUwotU2hvdWxkIG1vdmUgZnJvbSBpbnB1dC1BLTEgdG8gaW5wdXQtRC0wIGluIGJhY2t3 YXJkCitTaG91bGQgbW92ZSBmcm9tIGhvc3QtRCB0byBpbnB1dC1FLTAgaW4gZm9yd2FyZAorUEFT UworU2hvdWxkIG1vdmUgZnJvbSBpbnB1dC1FLTAgdG8gaW5wdXQtQS0xIGluIGZvcndhcmQKK1BB U1MKK1Nob3VsZCBtb3ZlIGZyb20gaW5wdXQtQS0xIHRvIGlucHV0LUUtMCBpbiBiYWNrd2FyZAor UEFTUworU2hvdWxkIG1vdmUgZnJvbSBpbnB1dC1FLTAgdG8gaG9zdC1EIGluIGJhY2t3YXJkCitQ QVNTCitTaG91bGQgbW92ZSBmcm9tIGhvc3QtRCB0byBpbnB1dC1ELTAgaW4gYmFja3dhcmQKIFBB U1MKIFNob3VsZCBtb3ZlIGZyb20gaW5wdXQtRC0wIHRvIGlucHV0LUMtMCBpbiBiYWNrd2FyZAog UEFTUwpAQCAtOTYsNyArMTA0LDcgQEAgU2hvdWxkIG1vdmUgZnJvbSBpbnB1dC1DLTEgdG8gaW5w dXQtQi0xIGluIGJhY2t3YXJkCiBQQVNTCiBTaG91bGQgbW92ZSBmcm9tIGlucHV0LUItMSB0byBp bnB1dC1BLTEgaW4gYmFja3dhcmQKIFBBU1MKLVNob3VsZCBtb3ZlIGZyb20gaW5wdXQtQS0xIHRv IGlucHV0LUQtMCBpbiBiYWNrd2FyZAorU2hvdWxkIG1vdmUgZnJvbSBpbnB1dC1BLTEgdG8gaW5w dXQtRS0wIGluIGJhY2t3YXJkCiBQQVNTCiBUZXN0IGZpbmlzaGVkLgogCkBAIC0xMDQsMyArMTEy LDQgQEAgVGVzdCBmaW5pc2hlZC4KIAogCiAKKwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvZmFz dC9kb20vc2hhZG93L2ZvY3VzLW5hdmlnYXRpb24uaHRtbCBiL0xheW91dFRlc3RzL2Zhc3QvZG9t L3NoYWRvdy9mb2N1cy1uYXZpZ2F0aW9uLmh0bWwKaW5kZXggN2U3MDk1NzBkZWJlYzMyZDY5OGFh NjRmMWEyNmQyN2U2ZGZiZjg3Ni4uYzdiMWUxOGFmNGI5MWRmNTkxNDIwOTNjZDBhODk3ZTgzYWEx YTk5ZiAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvZmFzdC9kb20vc2hhZG93L2ZvY3VzLW5hdmln YXRpb24uaHRtbAorKysgYi9MYXlvdXRUZXN0cy9mYXN0L2RvbS9zaGFkb3cvZm9jdXMtbmF2aWdh dGlvbi5odG1sCkBAIC01MCw4ICs1MCwxMSBAQCBmdW5jdGlvbiBwcmVwYXJlRE9NVHJlZShwYXJl bnQpCiAgICAgICAgICAgICAgICAgICBjcmVhdGVET00oJ2RpdicsIHsnaWQnOiAnaG9zdC1DJywg J3RhYmluZGV4JzogLTF9LAogICAgICAgICAgICAgICAgICAgICAgICAgICAgIGNyZWF0ZVNoYWRv d1Jvb3QoCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGNyZWF0ZURPTSgnaW5wdXQn LCB7J3RhYmluZGV4JzogLTF9KSkpLAotICAgICAgICAgICAgICAgICAgY3JlYXRlRE9NKCdpbnB1 dCcsIHsnaWQnOiAnaW5wdXQtRC0wJywgJ3RhYmluZGV4JzogMH0pKSk7Ci0KKyAgICAgICAgICAg ICAgICAgIGNyZWF0ZURPTSgnaW5wdXQnLCB7J2lkJzogJ2lucHV0LUQtMCcsICd0YWJpbmRleCc6 IDB9KSwKKyAgICAgICAgICAgICAgICAgIGNyZWF0ZURPTSgnZGl2Jywge30sCisgICAgICAgICAg ICAgICAgICAgICAgICAgICAgY3JlYXRlRE9NKCdkaXYnLCB7J2lkJzogJ2hvc3QtRCcsICd0YWJp bmRleCc6IDB9LAorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjcmVhdGVT aGFkb3dSb290KCkpKSwKKyAgICAgICAgICAgICAgICAgIGNyZWF0ZURPTSgnaW5wdXQnLCB7J2lk JzogJ2lucHV0LUUtMCcsICd0YWJpbmRleCc6IDB9KSkpOwogICAgIHBhcmVudC5vZmZzZXRMZWZ0 OwogfQogCkBAIC0xMTcsNiArMTIwLDggQEAgZnVuY3Rpb24gdGVzdCgpIHsKIAogICAgICAgICAv LyBBIG5vbi1mb2N1c2FibGUgc2hhZG93IGhvc3QgKGlkPWhvc3QtQyksIHdoaWNoIGRvZXMgbm90 IGhhdmUgZm9jdXNhYmxlIGVsZW1lbnRzLCBzaG91bGQgYmUgc2tpcHBlZCBlbnRpcmVseS4KICAg ICAgICAgJ2lucHV0LUQtMCcsCisgICAgICAgICdob3N0LUQnLAorICAgICAgICAnaW5wdXQtRS0w JywKIAogICAgICAgICAvLyBXcmFwcyB0byB0aGUgZmlyc3QgZWxlbWVudCBpbiB0aGUgb3V0ZXJt b3N0IGZvY3VzIHNjb3BlLgogICAgICAgICAnaW5wdXQtQS0xJywKQEAgLTEyNSw2ICsxMzAsOCBA QCBmdW5jdGlvbiB0ZXN0KCkgewogCiAgICAgZWxlbWVudHNOYXZpZ2F0ZWRJbkJhY2t3YXJkID0g WwogICAgICAgICAnaW5wdXQtQS0xJywKKyAgICAgICAgJ2lucHV0LUUtMCcsCisgICAgICAgICdo b3N0LUQnLAogICAgICAgICAnaW5wdXQtRC0wJywKICAgICAgICAgJ2lucHV0LUMtMCcsCiAgICAg ICAgICdpbnB1dC1CLTAnLApAQCAtMTYzLDcgKzE3MCw3IEBAIGZ1bmN0aW9uIHRlc3QoKSB7CiAg ICAgICAgICdpbnB1dC1BLTEnLAogCiAgICAgICAgIC8vIFdyYXBzIHRvIHRoZSBsYXN0IGVsZW1l bnQgaW4gdGhlIG91dGVybW9zdCBmb2N1cyBzY29wZS4KLSAgICAgICAgJ2lucHV0LUQtMCcsCisg ICAgICAgICdpbnB1dC1FLTAnLAogICAgIF07CiAKICAgICB0ZXN0Rm9jdXNOYXZpZ2F0aW9uRm93 cmFkKGVsZW1lbnRzTmF2aWdhdGVkSW5Gb3J3YXJkKTsK