16749 2008-01-05 14:15:14 -0800 DOMRange.surroundContents throws wrong exception (Acid3 bug) 2019-02-06 09:04:09 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 17064 1 eric darin ap cdumez ian oldest_to_newest 66302 0 eric 2008-01-05 14:15:14 -0800 DOMRange.surroundContents throws wrong exception (Acid3 bug) This one should be easy to fix. :) function () { // test 15: Ranges and Comments var doc = getTestDocument(); var c1 = doc.createComment("aaaaa"); doc.appendChild(c1); var c2 = doc.createComment("bbbbb"); doc.appendChild(c2); var r = doc.createRange(); r.setStart(c1, 2); r.setEnd(c2, 3); var msg = 'wrong exception raised'; try { r.surroundContents(doc.createElement('a')); msg = 'no exception raised'; } catch (e) { if ('code' in e) msg += '; code = ' + e.code; if (e.code == e.BAD_BOUNDARYPOINTS_ERR) msg = ''; } assert(msg == '', msg); assert(r.toString() == "", "comments returned text"); return 1; }, function () { return 1; }, 66303 1 eric 2008-01-05 14:15:35 -0800 The failure text: Test 15: FAIL (wrong exception raised; code = 3) 66321 2 ap 2008-01-05 15:56:13 -0800 "BAD_BOUNDARYPOINTS_ERR: Raised if the Range partially selects a non-text node." I'm wondering why the spec says that - why are Text nodes different from Comment and CDATASection ones in this regard? 66341 3 18294 andrew 2008-01-05 20:27:41 -0800 Created attachment 18294 Proposed patch Ensure that we throw for all non-text nodes not just those that aren't character indexed for offsets. Also moved the code for this exception up the method so it'll throw before HIERARCHY_REQUEST_ERR, which will let this Acid3 test pass. 66345 4 18294 eric 2008-01-05 22:20:35 -0800 Comment on attachment 18294 Proposed patch A couple things. I'm not yet sure we really have enough test coverage for this change. Perhaps I'm just not thinking clear enough at this hour, or perhaps you can just convince me over IRC. Also, our style guidelines forbid c-style casts in c++ code: (int)m_endOffset 66346 5 eric 2008-01-05 22:39:58 -0800 Ian, I think it's unclear as to why this is the "wrong exception". 66350 6 ian 2008-01-05 22:56:03 -0800 woops, i meant to put the comment nodes in an element, not in a document. my bad. will fix. 66358 7 eric 2008-01-06 00:16:16 -0800 FYI the fail message is now: Test 15: FAIL (when trying to surround two halves of comment: wrong exception raised; code = 3) 66361 8 ap 2008-01-06 00:24:10 -0800 As Andrew has discovered, this case meets conditions for both exceptions. 66364 9 18294 darin 2008-01-06 01:05:35 -0800 Comment on attachment 18294 Proposed patch Looks good! + } else if (m_startContainer->nodeType() != Node::TEXT_NODE) { I would just have used isTextNode() here rather than nodeType(). + if (m_startOffset > 0) { + ec = RangeException::BAD_BOUNDARYPOINTS_ERR; + return; + } This seems wrong. If m_startOffset is >= m_startContainer->maxCharacterOffset, I think we're fine here. review- because of that incorrect case. We need to add a test where the start container is a comment node and the start offset is the end of the comment node. 66609 10 ian 2008-01-08 19:37:34 -0800 (please do let me know if you disagree with what the test currently does) 70103 11 darin 2008-02-10 12:10:17 -0800 (In reply to comment #10) > (please do let me know if you disagree with what the test currently does) I don't think the specification is clear enough about which exception will be raised when there are multiple things wrong. It's easy to make WebKit match Acid3's expectations and we'll do that. But the specification should be fixed to be specific enough; there might be additional WebKit fixes needed then. Where would we do that? In HTML 5? 70104 12 darin 2008-02-10 12:14:25 -0800 Committed revision 30123. 70121 13 ian 2008-02-10 13:17:37 -0800 This would be a WebAPI working group thing. We need a volunteer to edit the spec. 1503110 14 lforschler 2019-02-06 09:04:09 -0800 Mass moving XML DOM bugs to the "DOM" Component. 18294 2008-01-05 20:27:41 -0800 2008-01-06 01:05:35 -0800 Proposed patch 16749-patch-1.txt text/plain 5379 andrew SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAyOTIwMykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDgtMDEtMDUgIEFuZHJldyBXZWxsaW5ndG9uICA8cHJvdG9uQHdp cmV0YXBwZWQubmV0PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorICAg ICAgICAKKyAgICAgICAgRE9NUmFuZ2Uuc3Vycm91bmRDb250ZW50cyB0aHJvd3Mgd3JvbmcgZXhj ZXB0aW9uIChBY2lkMyBidWcpCisgICAgICAgIGh0dHA6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE2NzQ5CisgICAgICAgIAorICAgICAgICBUaHJvdyBCQURfQk9VTkRBUllQT0lO VFNfRVJSIGlmIGF0dGVtcHRpbmcgdG8gc3BsaXQgYSBub24tdGV4dCBub2RlIHRoYXQKKyAgICAg ICAgaGFzIG9mZnNldHMgaW4gY2hhcmFjdGVycworCisgICAgICAgICogZG9tL1JhbmdlLmNwcDoK KyAgICAgICAgKFdlYkNvcmU6OlJhbmdlOjpzdXJyb3VuZENvbnRlbnRzKToKKwogMjAwOC0wMS0w NSAgU2FtIFdlaW5pZyAgPHNhbUB3ZWJraXQub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEVy aWMgU2VpZGVsLgpJbmRleDogV2ViQ29yZS9kb20vUmFuZ2UuY3BwCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdl YkNvcmUvZG9tL1JhbmdlLmNwcAkocmV2aXNpb24gMjkyMDMpCisrKyBXZWJDb3JlL2RvbS9SYW5n ZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE0MDEsNiArMTQwMSwzMCBAQCB2b2lkIFJhbmdlOjpz dXJyb3VuZENvbnRlbnRzKFBhc3NSZWZQdHI8CiAgICAgICAgIGVjID0gV1JPTkdfRE9DVU1FTlRf RVJSOwogICAgICAgICByZXR1cm47CiAgICAgfQorICAgIAorICAgIC8vIEJBRF9CT1VOREFSWVBP SU5UU19FUlI6IFJhaXNlZCBpZiB0aGUgUmFuZ2UgcGFydGlhbGx5IHNlbGVjdHMgYSBub24tdGV4 dCBub2RlLgorICAgIGlmICghbV9zdGFydENvbnRhaW5lci0+b2Zmc2V0SW5DaGFyYWN0ZXJzKCkp IHsKKyAgICAgICAgaWYgKG1fc3RhcnRPZmZzZXQgPiAwICYmIG1fc3RhcnRPZmZzZXQgPCBtX3N0 YXJ0Q29udGFpbmVyLT5jaGlsZE5vZGVDb3VudCgpKSB7CisgICAgICAgICAgICBlYyA9IFJhbmdl RXhjZXB0aW9uOjpCQURfQk9VTkRBUllQT0lOVFNfRVJSOworICAgICAgICAgICAgcmV0dXJuOwor ICAgICAgICB9CisgICAgfSBlbHNlIGlmIChtX3N0YXJ0Q29udGFpbmVyLT5ub2RlVHlwZSgpICE9 IE5vZGU6OlRFWFRfTk9ERSkgeworICAgICAgICBpZiAobV9zdGFydE9mZnNldCA+IDApIHsKKyAg ICAgICAgICAgIGVjID0gUmFuZ2VFeGNlcHRpb246OkJBRF9CT1VOREFSWVBPSU5UU19FUlI7Cisg ICAgICAgICAgICByZXR1cm47CisgICAgICAgIH0KKyAgICB9CisgICAgaWYgKCFtX2VuZENvbnRh aW5lci0+b2Zmc2V0SW5DaGFyYWN0ZXJzKCkpIHsKKyAgICAgICAgaWYgKG1fZW5kT2Zmc2V0ID4g MCAmJiBtX2VuZE9mZnNldCA8IG1fZW5kQ29udGFpbmVyLT5jaGlsZE5vZGVDb3VudCgpKSB7Cisg ICAgICAgICAgICBlYyA9IFJhbmdlRXhjZXB0aW9uOjpCQURfQk9VTkRBUllQT0lOVFNfRVJSOwor ICAgICAgICAgICAgcmV0dXJuOworICAgICAgICB9CisgICAgfSBlbHNlIGlmIChtX2VuZENvbnRh aW5lci0+bm9kZVR5cGUoKSAhPSBOb2RlOjpURVhUX05PREUpIHsKKyAgICAgICAgaWYgKG1fZW5k T2Zmc2V0ID4gMCAmJiAoaW50KW1fZW5kT2Zmc2V0IDwgbV9lbmRDb250YWluZXItPm1heENoYXJh Y3Rlck9mZnNldCgpKSB7CisgICAgICAgICAgICBlYyA9IFJhbmdlRXhjZXB0aW9uOjpCQURfQk9V TkRBUllQT0lOVFNfRVJSOworICAgICAgICAgICAgcmV0dXJuOworICAgICAgICB9CisgICAgfSAg ICAKIAogICAgIC8vIFJhaXNlIGEgSElFUkFSQ0hZX1JFUVVFU1RfRVJSIGlmIG1fc3RhcnRDb250 YWluZXIgZG9lc24ndCBhY2NlcHQgY2hpbGRyZW4gbGlrZSBuZXdQYXJlbnQuCiAgICAgTm9kZSog cGFyZW50T2ZOZXdQYXJlbnQgPSBtX3N0YXJ0Q29udGFpbmVyLmdldCgpOwpAQCAtMTQyMCwyMCAr MTQ0NCw2IEBAIHZvaWQgUmFuZ2U6OnN1cnJvdW5kQ29udGVudHMoUGFzc1JlZlB0cjwKIAogICAg IC8vICMjIyBjaGVjayBpZiBub2RlIHdvdWxkIGVuZCB1cCB3aXRoIGEgY2hpbGQgbm9kZSBvZiBh IHR5cGUgbm90IGFsbG93ZWQgYnkgdGhlIHR5cGUgb2Ygbm9kZQogCi0gICAgLy8gQkFEX0JPVU5E QVJZUE9JTlRTX0VSUjogUmFpc2VkIGlmIHRoZSBSYW5nZSBwYXJ0aWFsbHkgc2VsZWN0cyBhIG5v bi10ZXh0IG5vZGUuCi0gICAgaWYgKCFtX3N0YXJ0Q29udGFpbmVyLT5vZmZzZXRJbkNoYXJhY3Rl cnMoKSkgewotICAgICAgICBpZiAobV9zdGFydE9mZnNldCA+IDAgJiYgbV9zdGFydE9mZnNldCA8 IG1fc3RhcnRDb250YWluZXItPmNoaWxkTm9kZUNvdW50KCkpIHsKLSAgICAgICAgICAgIGVjID0g UmFuZ2VFeGNlcHRpb246OkJBRF9CT1VOREFSWVBPSU5UU19FUlI7Ci0gICAgICAgICAgICByZXR1 cm47Ci0gICAgICAgIH0KLSAgICB9Ci0gICAgaWYgKCFtX2VuZENvbnRhaW5lci0+b2Zmc2V0SW5D aGFyYWN0ZXJzKCkpIHsKLSAgICAgICAgaWYgKG1fZW5kT2Zmc2V0ID4gMCAmJiBtX2VuZE9mZnNl dCA8IG1fZW5kQ29udGFpbmVyLT5jaGlsZE5vZGVDb3VudCgpKSB7Ci0gICAgICAgICAgICBlYyA9 IFJhbmdlRXhjZXB0aW9uOjpCQURfQk9VTkRBUllQT0lOVFNfRVJSOwotICAgICAgICAgICAgcmV0 dXJuOwotICAgICAgICB9Ci0gICAgfQotCiAgICAgZWMgPSAwOwogICAgIHdoaWxlIChOb2RlKiBu ID0gbmV3UGFyZW50LT5maXJzdENoaWxkKCkpIHsKICAgICAgICAgbmV3UGFyZW50LT5yZW1vdmVD aGlsZChuLCBlYyk7CkluZGV4OiBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g TGF5b3V0VGVzdHMvQ2hhbmdlTG9nCShyZXZpc2lvbiAyOTIwMykKKysrIExheW91dFRlc3RzL0No YW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDA4LTAxLTA1ICBBbmRy ZXcgV2VsbGluZ3RvbiAgPHByb3RvbkB3aXJldGFwcGVkLm5ldD4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKyAgICAgICAgCisgICAgICAgIERPTVJhbmdlLnN1cnJvdW5k Q29udGVudHMgdGhyb3dzIHdyb25nIGV4Y2VwdGlvbiAoQWNpZDMgYnVnKQorICAgICAgICBodHRw Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjc0OQorICAgICAgICAKKyAgICAg ICAgVGhyb3cgQkFEX0JPVU5EQVJZUE9JTlRTX0VSUiBpZiBhdHRlbXB0aW5nIHRvIHNwbGl0IGEg bm9uLXRleHQgbm9kZSB0aGF0CisgICAgICAgIGhhcyBvZmZzZXRzIGluIGNoYXJhY3RlcnMKKwor ICAgICAgICAqIGZhc3QvZG9tL1JhbmdlL3JhbmdlLWV4Y2VwdGlvbnMtZXhwZWN0ZWQudHh0Ogor ICAgICAgICAqIGZhc3QvZG9tL1JhbmdlL3Jlc291cmNlcy9yYW5nZS1leGNlcHRpb25zLmpzOgor CiAyMDA4LTAxLTA1ICBTYW0gV2VpbmlnICA8c2FtQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2 aWV3ZWQgYnkgRXJpYyBTZWlkZWwuCkluZGV4OiBMYXlvdXRUZXN0cy9mYXN0L2RvbS9SYW5nZS9y YW5nZS1leGNlcHRpb25zLWV4cGVjdGVkLnR4dAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9m YXN0L2RvbS9SYW5nZS9yYW5nZS1leGNlcHRpb25zLWV4cGVjdGVkLnR4dAkocmV2aXNpb24gMjky MDMpCisrKyBMYXlvdXRUZXN0cy9mYXN0L2RvbS9SYW5nZS9yYW5nZS1leGNlcHRpb25zLWV4cGVj dGVkLnR4dAkod29ya2luZyBjb3B5KQpAQCAtNiw2ICs2LDcgQEAgT24gc3VjY2VzcywgeW91IHdp bGwgc2VlIGEgc2VyaWVzIG9mICJQQQogUEFTUyBub2RlLmlubmVySFRNTCBpcyAnPGJhcj5BQjxt b28+QzwvbW9vPkRFPC9iYXI+JwogUEFTUyBmb28ub3V0ZXJIVE1MIGlzICc8Zm9vPjwvZm9vPicK IFBBU1MgcmFuZ2Uuc3Vycm91bmRDb250ZW50cyhmb28pIHRocmV3IGV4Y2VwdGlvbiBFcnJvcjog QkFEX0JPVU5EQVJZUE9JTlRTX0VSUjogRE9NIFJhbmdlIEV4Y2VwdGlvbiAxLgorUEFTUyByLnN1 cnJvdW5kQ29udGVudHMoZG9jdW1lbnQuY3JlYXRlRWxlbWVudCgnYScpKSB0aHJldyBleGNlcHRp b24gRXJyb3I6IEJBRF9CT1VOREFSWVBPSU5UU19FUlI6IERPTSBSYW5nZSBFeGNlcHRpb24gMS4K IFBBU1Mgc3VjY2Vzc2Z1bGx5UGFyc2VkIGlzIHRydWUKIAogVEVTVCBDT01QTEVURQpJbmRleDog TGF5b3V0VGVzdHMvZmFzdC9kb20vUmFuZ2UvcmVzb3VyY2VzL3JhbmdlLWV4Y2VwdGlvbnMuanMK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvZmFzdC9kb20vUmFuZ2UvcmVzb3VyY2VzL3Jhbmdl LWV4Y2VwdGlvbnMuanMJKHJldmlzaW9uIDI5MjAzKQorKysgTGF5b3V0VGVzdHMvZmFzdC9kb20v UmFuZ2UvcmVzb3VyY2VzL3JhbmdlLWV4Y2VwdGlvbnMuanMJKHdvcmtpbmcgY29weSkKQEAgLTEz LDQgKzEzLDE5IEBAIHZhciBmb28gPSBkb2N1bWVudC5jcmVhdGVFbGVtZW50KCJGT08iKTsKIHNo b3VsZEJlKCJmb28ub3V0ZXJIVE1MIiwgIic8Zm9vPjwvZm9vPiciKTsKIHNob3VsZFRocm93KCJy YW5nZS5zdXJyb3VuZENvbnRlbnRzKGZvbykiKTsKIAorLy8gRW5zdXJlIHRoYXQgd2UgdGhyb3cg Zm9yIHRyeWluZyB0byBzcGxpdCBhIGNvbW1lbnQKKy8vIChub24tdGV4dCBidXQgY2hhcmFjdGVy IG9mZnNldCBub2RlKS4KKy8vIAorLy8gVGVzdCBhZGFwdGVkIGZyb20gQWNpZDMKK3ZhciBjMSA9 IGRvY3VtZW50LmNyZWF0ZUNvbW1lbnQoImFhYWFhIik7Citkb2N1bWVudC5hcHBlbmRDaGlsZChj MSk7Cit2YXIgYzIgPSBkb2N1bWVudC5jcmVhdGVDb21tZW50KCJiYmJiYiIpOworZG9jdW1lbnQu YXBwZW5kQ2hpbGQoYzIpOwordmFyIHIgPSBkb2N1bWVudC5jcmVhdGVSYW5nZSgpOworci5zZXRT dGFydChjMSwgMik7CityLnNldEVuZChjMiwgMyk7CitzaG91bGRUaHJvdygici5zdXJyb3VuZENv bnRlbnRzKGRvY3VtZW50LmNyZWF0ZUVsZW1lbnQoJ2EnKSkiLCAnIkVycm9yOiBCQURfQk9VTkRB UllQT0lOVFNfRVJSOiBET00gUmFuZ2UgRXhjZXB0aW9uIDEiJyk7CisKKworCiB2YXIgc3VjY2Vz c2Z1bGx5UGFyc2VkID0gdHJ1ZTsK