76146 2012-01-11 21:33:26 -0800 NodeIterator loses track of the reference node when the reference node is removed from the document (IETC ni_removeReferenceNode) 2012-01-12 15:18:56 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 76198 1 abarth abarth ap darin eric webkit.review.bot oldest_to_newest 534560 0 abarth 2012-01-11 21:33:26 -0800 NodeIterator loses track of the reference node when the reference node is removed from the document (IETC ni_removeReferenceNode) 534563 1 122167 abarth 2012-01-11 21:38:49 -0800 Created attachment 122167 Patch 534571 2 122167 rniwa 2012-01-11 22:20:26 -0800 Comment on attachment 122167 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122167&action=review > Source/WebCore/dom/NodeIterator.cpp:185 > + while (node && node->isDescendantOf(removedNode)) > + node = node->traverseNextNode(root()); This is every inefficient way to find the next node. If node is a descendant of the removed node, then the next node should be removedNode->traverseNextSibling(), no? 534583 3 abarth 2012-01-11 23:02:26 -0800 @darin: I've been told that you're the expert on this code. This function is pretty subtle. If you have a few minutes to sanity-check this patch, that would be great. Thanks! 534986 4 122167 darin 2012-01-12 11:11:36 -0800 Comment on attachment 122167 Patch I did fix quite a few bugs in this code a while back. The change in the code looks fine. I think to improve performance and correctness we’d want to add performance and correctness tests. This little-used barely-useful obscure corner of the DOM does not get a lot of attention or love. 535050 5 abarth 2012-01-12 12:17:27 -0800 It looks like DOM4 has a reasonably simple specification of what to do when nodes are removed: http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#interface-nodeiterator I wonder if we can re-work our implementation to match the spec more closely. 535139 6 122167 abarth 2012-01-12 14:05:20 -0800 Comment on attachment 122167 Patch I'm going to work on refactoring and adding more tests for NodeIterator in future bug. 535218 7 122167 webkit.review.bot 2012-01-12 15:18:51 -0800 Comment on attachment 122167 Patch Clearing flags on attachment: 122167 Committed r104866: <http://trac.webkit.org/changeset/104866> 535219 8 webkit.review.bot 2012-01-12 15:18:56 -0800 All reviewed patches have been landed. Closing bug. 122167 2012-01-11 21:38:49 -0800 2012-01-12 15:18:51 -0800 Patch bug-76146-20120111213848.patch text/plain 5988 abarth SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwNDc4NykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDMwIEBACisyMDEyLTAxLTExICBBZGFtIEJh cnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CisKKyAgICAgICAgTm9kZUl0ZXJhdG9yIGxvc2VzIHRy YWNrIG9mIHRoZSByZWZlcmVuY2Ugbm9kZSB3aGVuIHRoZSByZWZlcmVuY2Ugbm9kZSBpcyByZW1v dmVkIGZyb20gdGhlIGRvY3VtZW50IChJRVRDIG5pX3JlbW92ZVJlZmVyZW5jZU5vZGUpCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03NjE0NgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEluIHRoZSBjYXNlIHdo ZXJlIHdlJ3JlIHJlbW92aW5nIHRoZSByZWZlcmVuY2Ugbm9kZSB3ZSBjYW4gZW5kIHVwIHdpdGgK KyAgICAgICAgdGhlIHdyb25nIHJlZmVyZW5jZSBub2RlLiAgVGhpcyBwYXRjaCBtYWtlcyBzdXJl IHdlIHRyYXZlcnNlIG91dHNpZGUgb2YKKyAgICAgICAgdGhlIHJlbW92ZWQgbm9kZSdzIHN1YnRy ZWUuCisKKyAgICAgICAgVGhpcyBidWcgd2FzIGNhdWdodCBieSB0aGUgZm9sbG93aW5nIElFIFRl c3QgQ2VudGVyIHRlc3Q6CisKKyAgICAgICAgaHR0cDovL3NhbXBsZXMubXNkbi5taWNyb3NvZnQu Y29tL2lldGVzdGNlbnRlci9kb210cmF2ZXJzYWwvc2hvd2RvbXRyYXZlcnNhbHRlc3QuaHRtP25p X3JlbW92ZVJlZmVyZW5jZU5vZGUKKworICAgICAgICBPdXIgbmV3IGJlaGF2aW9yIGFsc28gbWF0 Y2ggRmlyZWZveC4KKworICAgICAgICBJIGV4cGVyaW1lbnRlZCBhIGJpdCB3aXRoIGFkZGluZyBB U1NFUlRfTk9UX1JFQUNIRUQgdG8gdmFyaW91cyBicmFuY2hlcworICAgICAgICBpbiBOb2RlSXRl cmF0b3I6OnVwZGF0ZUZvck5vZGVSZW1vdmFsLCBhbmQgaXQgc2VlbXMgb3VyIHRlc3QgY292ZXJh Z2UKKyAgICAgICAgZm9yIHRoaXMgZnVuY3Rpb24gaXMgcmVsYXRpdmVseSBwb29yLiAgSW4gdGhl IGZ1dHVyZSwgd2Ugc2hvdWxkCisgICAgICAgIGNvbnNpZGVyIGFkZGluZyBtb3JlIHRlc3RzIGZv ciB0aGlzIGNvbXBsaWNhdGVkIGZ1bmN0aW9uLgorCisgICAgICAgIFRlc3Q6IGZhc3QvZG9tL25v ZGUtaXRlcmF0b3ItcmVmZXJlbmNlLW5vZGUtcmVtb3ZlZC5odG1sCisKKyAgICAgICAgKiBkb20v Tm9kZUl0ZXJhdG9yLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok5vZGVJdGVyYXRvcjo6dXBkYXRl Rm9yTm9kZVJlbW92YWwpOgorCiAyMDEyLTAxLTExICBEYW4gQmVybnN0ZWluICA8bWl0ekBhcHBs ZS5jb20+CiAKICAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzEwNjc5MDM1PiBJbXBsZW1lbnQgZm9u dC12YXJpYW50LWxpZ2F0dXJlczoge25vLX1jb21tb24tbGlnYXR1cmVzCkluZGV4OiBTb3VyY2Uv V2ViQ29yZS9kb20vTm9kZUl0ZXJhdG9yLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29y ZS9kb20vTm9kZUl0ZXJhdG9yLmNwcAkocmV2aXNpb24gMTA0NzM4KQorKysgU291cmNlL1dlYkNv cmUvZG9tL05vZGVJdGVyYXRvci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE3OSwxMiArMTc5LDEw IEBAIHZvaWQgTm9kZUl0ZXJhdG9yOjp1cGRhdGVGb3JOb2RlUmVtb3ZhbCgKICAgICBpZiAocmVm ZXJlbmNlTm9kZS5pc1BvaW50ZXJCZWZvcmVOb2RlKSB7CiAgICAgICAgIE5vZGUqIG5vZGUgPSBy ZW1vdmVkTm9kZS0+dHJhdmVyc2VOZXh0Tm9kZShyb290KCkpOwogICAgICAgICBpZiAobm9kZSkg ewotICAgICAgICAgICAgLy8gTW92ZSBvdXQgZnJvbSB1bmRlciB0aGUgbm9kZSBiZWluZyByZW1v dmVkIGlmIHRoZSByZWZlcmVuY2Ugbm9kZSBpcwotICAgICAgICAgICAgLy8gYSBkZXNjZW5kYW50 IG9mIHRoZSBub2RlIGJlaW5nIHJlbW92ZWQuCi0gICAgICAgICAgICBpZiAod2lsbFJlbW92ZVJl ZmVyZW5jZU5vZGVBbmNlc3RvcikgewotICAgICAgICAgICAgICAgIHdoaWxlIChub2RlICYmIG5v ZGUtPmlzRGVzY2VuZGFudE9mKHJlbW92ZWROb2RlKSkKLSAgICAgICAgICAgICAgICAgICAgbm9k ZSA9IG5vZGUtPnRyYXZlcnNlTmV4dE5vZGUocm9vdCgpKTsKLSAgICAgICAgICAgIH0KKyAgICAg ICAgICAgIC8vIE1vdmUgb3V0IGZyb20gdW5kZXIgdGhlIG5vZGUgYmVpbmcgcmVtb3ZlZCBpZiB0 aGUgbmV3IHJlZmVyZW5jZQorICAgICAgICAgICAgLy8gbm9kZSBpcyBhIGRlc2NlbmRhbnQgb2Yg dGhlIG5vZGUgYmVpbmcgcmVtb3ZlZC4KKyAgICAgICAgICAgIHdoaWxlIChub2RlICYmIG5vZGUt PmlzRGVzY2VuZGFudE9mKHJlbW92ZWROb2RlKSkKKyAgICAgICAgICAgICAgICBub2RlID0gbm9k ZS0+dHJhdmVyc2VOZXh0Tm9kZShyb290KCkpOwogICAgICAgICAgICAgaWYgKG5vZGUpCiAgICAg ICAgICAgICAgICAgcmVmZXJlbmNlTm9kZS5ub2RlID0gbm9kZTsKICAgICAgICAgfSBlbHNlIHsK QEAgLTIxNyw2ICsyMTUsNyBAQCB2b2lkIE5vZGVJdGVyYXRvcjo6dXBkYXRlRm9yTm9kZVJlbW92 YWwoCiAgICAgICAgICAgICBpZiAobm9kZSkKICAgICAgICAgICAgICAgICByZWZlcmVuY2VOb2Rl Lm5vZGUgPSBub2RlOwogICAgICAgICB9IGVsc2UgeworICAgICAgICAgICAgLy8gRklYTUU6IFRo aXMgYnJhbmNoIGRvZXNuJ3QgYXBwZWFyIHRvIGhhdmUgYW55IExheW91dFRlc3RzLgogICAgICAg ICAgICAgbm9kZSA9IHJlbW92ZWROb2RlLT50cmF2ZXJzZU5leHROb2RlKHJvb3QoKSk7CiAgICAg ICAgICAgICAvLyBNb3ZlIG91dCBmcm9tIHVuZGVyIHRoZSBub2RlIGJlaW5nIHJlbW92ZWQgaWYg dGhlIHJlZmVyZW5jZSBub2RlIGlzCiAgICAgICAgICAgICAvLyBhIGRlc2NlbmRhbnQgb2YgdGhl IG5vZGUgYmVpbmcgcmVtb3ZlZC4KSW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwNDc4NykKKysrIExheW91 dFRlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDEyLTAx LTExICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CisKKyAgICAgICAgTm9kZUl0ZXJh dG9yIGxvc2VzIHRyYWNrIG9mIHRoZSByZWZlcmVuY2Ugbm9kZSB3aGVuIHRoZSByZWZlcmVuY2Ug bm9kZSBpcyByZW1vdmVkIGZyb20gdGhlIGRvY3VtZW50IChJRVRDIG5pX3JlbW92ZVJlZmVyZW5j ZU5vZGUpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03 NjE0NgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRl c3QgYmFzZWQgb24gaHR0cDovL3NhbXBsZXMubXNkbi5taWNyb3NvZnQuY29tL2lldGVzdGNlbnRl ci9kb210cmF2ZXJzYWwvc2hvd2RvbXRyYXZlcnNhbHRlc3QuaHRtP25pX3JlbW92ZVJlZmVyZW5j ZU5vZGUKKworICAgICAgICAqIGZhc3QvZG9tL25vZGUtaXRlcmF0b3ItcmVmZXJlbmNlLW5vZGUt cmVtb3ZlZC1leHBlY3RlZC50eHQ6IEFkZGVkLgorICAgICAgICAqIGZhc3QvZG9tL25vZGUtaXRl cmF0b3ItcmVmZXJlbmNlLW5vZGUtcmVtb3ZlZC5odG1sOiBBZGRlZC4KKwogMjAxMi0wMS0xMSAg RGFuIEJlcm5zdGVpbiAgPG1pdHpAYXBwbGUuY29tPgogCiAgICAgICAgIDxyZGFyOi8vcHJvYmxl bS8xMDY3OTAzNT4gSW1wbGVtZW50IGZvbnQtdmFyaWFudC1saWdhdHVyZXM6IHtuby19Y29tbW9u LWxpZ2F0dXJlcwpJbmRleDogTGF5b3V0VGVzdHMvZmFzdC9kb20vbm9kZS1pdGVyYXRvci1yZWZl cmVuY2Utbm9kZS1yZW1vdmVkLWV4cGVjdGVkLnR4dAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0 cy9mYXN0L2RvbS9ub2RlLWl0ZXJhdG9yLXJlZmVyZW5jZS1ub2RlLXJlbW92ZWQtZXhwZWN0ZWQu dHh0CShyZXZpc2lvbiAwKQorKysgTGF5b3V0VGVzdHMvZmFzdC9kb20vbm9kZS1pdGVyYXRvci1y ZWZlcmVuY2Utbm9kZS1yZW1vdmVkLWV4cGVjdGVkLnR4dAkocmV2aXNpb24gMCkKQEAgLTAsMCAr MSwxNiBAQAorVGhpcyB0ZXN0IHJlbW92ZXMgdGhlIE5vZGVJdGVydG9yJ3MgY3VycmVudCByZWZl cmVuY2Ugbm9kZSwgdGhlbiBjb250aW51ZXMgdG8gbW92ZSB0aHJvdWdoIHRoZSBkb2N1bWVudC4K KworT24gc3VjY2VzcywgeW91IHdpbGwgc2VlIGEgc2VyaWVzIG9mICJQQVNTIiBtZXNzYWdlcywg Zm9sbG93ZWQgYnkgIlRFU1QgQ09NUExFVEUiLgorCisKK1BBU1MgaXRlci5uZXh0Tm9kZSgpIGlz IHRlc3REaXYKK1BBU1MgaXRlci5uZXh0Tm9kZSgpIGlzIGRpdjEKK1BBU1MgaXRlci5uZXh0Tm9k ZSgpIGlzIGRpdjIKK1BBU1MgaXRlci5uZXh0Tm9kZSgpIGlzIG51bGwKK1BBU1MgaXRlci5wcmV2 aW91c05vZGUoKSBpcyBkaXYyCitQQVNTIGl0ZXIucHJldmlvdXNOb2RlKCkgaXMgZGl2MQorUEFT UyBpdGVyLm5leHROb2RlKCkgaXMgZGl2MgorUEFTUyBzdWNjZXNzZnVsbHlQYXJzZWQgaXMgdHJ1 ZQorCitURVNUIENPTVBMRVRFCisKSW5kZXg6IExheW91dFRlc3RzL2Zhc3QvZG9tL25vZGUtaXRl cmF0b3ItcmVmZXJlbmNlLW5vZGUtcmVtb3ZlZC5odG1sCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRl c3RzL2Zhc3QvZG9tL25vZGUtaXRlcmF0b3ItcmVmZXJlbmNlLW5vZGUtcmVtb3ZlZC5odG1sCShy ZXZpc2lvbiAwKQorKysgTGF5b3V0VGVzdHMvZmFzdC9kb20vbm9kZS1pdGVyYXRvci1yZWZlcmVu Y2Utbm9kZS1yZW1vdmVkLmh0bWwJKHJldmlzaW9uIDApCkBAIC0wLDAgKzEsMjQgQEAKKzwhRE9D VFlQRSBodG1sPgorPHNjcmlwdCBzcmM9Ii4uL2pzL3Jlc291cmNlcy9qcy10ZXN0LXByZS5qcyI+ PC9zY3JpcHQ+Cis8ZGl2IGlkPSJ0ZXN0RGl2IiBzdHlsZT0iZGlzcGxheTpub25lOyI+CisgICAg PGRpdiBpZD0iZGl2MSI+YWFhPC9kaXY+CisgICAgPGRpdiBpZD0iZGl2MiI+YmJiPC9kaXY+Cis8 L2Rpdj4KKzxzY3JpcHQ+CitkZXNjcmlwdGlvbigiVGhpcyB0ZXN0IHJlbW92ZXMgdGhlIE5vZGVJ dGVydG9yJ3MgY3VycmVudCByZWZlcmVuY2Ugbm9kZSwgdGhlbiBjb250aW51ZXMgdG8gbW92ZSB0 aHJvdWdoIHRoZSBkb2N1bWVudC4iKTsKKwordmFyIHRlc3REaXYgPSBkb2N1bWVudC5nZXRFbGVt ZW50QnlJZCgidGVzdERpdiIpOwordmFyIGRpdjEgPSBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgi ZGl2MSIpOwordmFyIGRpdjIgPSBkb2N1bWVudC5nZXRFbGVtZW50QnlJZCgiZGl2MiIpOwordmFy IGl0ZXIgPSBkb2N1bWVudC5jcmVhdGVOb2RlSXRlcmF0b3IodGVzdERpdiwgTm9kZUZpbHRlci5T SE9XX0VMRU1FTlQsIG51bGwsIGZhbHNlKTsKKworc2hvdWxkQmUoIml0ZXIubmV4dE5vZGUoKSIs ICJ0ZXN0RGl2Iik7CitzaG91bGRCZSgiaXRlci5uZXh0Tm9kZSgpIiwgImRpdjEiKTsKK3Nob3Vs ZEJlKCJpdGVyLm5leHROb2RlKCkiLCAiZGl2MiIpOworc2hvdWxkQmUoIml0ZXIubmV4dE5vZGUo KSIsICJudWxsIik7CitzaG91bGRCZSgiaXRlci5wcmV2aW91c05vZGUoKSIsICJkaXYyIik7Citz aG91bGRCZSgiaXRlci5wcmV2aW91c05vZGUoKSIsICJkaXYxIik7Cit0ZXN0RGl2LnJlbW92ZUNo aWxkKGRpdjEpOworc2hvdWxkQmUoIml0ZXIubmV4dE5vZGUoKSIsICJkaXYyIik7Cis8L3Njcmlw dD4KKzxzY3JpcHQgc3JjPSIuLi9qcy9yZXNvdXJjZXMvanMtdGVzdC1wb3N0LmpzIj48L3Njcmlw dD4K