24883 2009-03-27 06:50:56 -0700 Bad success test in parseXMLDocumentFragment in XMLTokenizerLibxml2.cpp 2009-05-04 17:22:23 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) All OS X 10.5 RESOLVED FIXED P2 Normal --- 1 kai webkit-unassigned oldest_to_newest 115536 0 kai 2009-03-27 06:50:56 -0700 parseXMLDocumentFragment in XMLTokenizerLibxml2.cpp contains the following clause: if (bytesProcessed == -1 || ((unsigned long)bytesProcessed) == sizeof(UChar) * chunk.length()) return false; The second term probably meant to test for !=. As it stands, this term almost never evaluates to true due to a second mistake: 'chunk' is UTF8, therefore multiplying with sizeof(UChar) is wrong. I hit a case in which the term evaluates to true and thereby makes the function fail: if 'chunk' contains a single non breaking space (or any other character which use two bytes in UTF8). 115540 1 29010 kai 2009-03-27 07:07:55 -0700 Created attachment 29010 Fix with test case 119805 2 29010 eric 2009-05-04 01:43:35 -0700 Comment on attachment 29010 Fix with test case This looks correct to me. Thanks for the patch and layout test! 119806 3 29010 eric 2009-05-04 01:46:28 -0700 Comment on attachment 29010 Fix with test case Actually, I should be more clear: This patch is fantastic. You clearly read and understood the instructions on how to submit a patch! I look forward to reviewing more patches from you soon! A few nits: 1. You are missing EMAIL_ADDRESS on your LayoutTest Changelog 2. We probably don't want these: Added: svn:eol-style + native if anything we want LF (unix style) 3. The test would be slightly better if it just output PASS or FAIL instead of the actual dumped chars: +This test checks to see if setting innerHTML to a single non-breaking space works. +Original: Â +Result: Â Otherwise, again, this is a fantastic patch. Very easy to review! 119902 4 eric 2009-05-04 17:22:23 -0700 Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/fast/innerHTML/innerHTML-nbsp-expected.txt A LayoutTests/fast/innerHTML/innerHTML-nbsp.xhtml M WebCore/ChangeLog M WebCore/dom/XMLTokenizerLibxml2.cpp Committed r43195 I modified the test case slightly when landing so that it says PASS/FAIL: http://trac.webkit.org/changeset/43195 29010 2009-03-27 07:07:55 -0700 2009-05-04 01:43:35 -0700 Fix with test case parseXMLDocumentFragment_patch.txt text/plain 4299 kai SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0MjAzMikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTcgQEAKKzIwMDktMDMtMjcgIEthaSBCcsO8bmluZyAgPGthaUBncmFudXMubmV0 PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorICAgICAgICAKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTI0ODgzCisgICAgICAg IDI0ODgzOiBCYWQgc3VjY2VzcyB0ZXN0IGluIHBhcnNlWE1MRG9jdW1lbnRGcmFnbWVudCBpbiBY TUxUb2tlbml6ZXJMaWJ4bWwyLmNwcAorICAgICAgICAKKyAgICAgICAgRml4ZWQgdGVzdCB3aGV0 aGVyIGFsbCB0aGUgY2h1bmsgaGFzIGJlZW4gcHJvY2Vzc2VkIHRvIGNvcnJlY3RseSBjb3VudCB1 dGY4IGJ5dGVzLgorCisgICAgICAgIFRlc3Q6IGZhc3QvaW5uZXJIVE1ML2lubmVySFRNTC1uYnNw LnhodG1sCisKKyAgICAgICAgKiBkb20vWE1MVG9rZW5pemVyTGlieG1sMi5jcHA6CisgICAgICAg IChXZWJDb3JlOjpwYXJzZVhNTERvY3VtZW50RnJhZ21lbnQpOgorCiAyMDA5LTAzLTI2ICBNYXJr IFJvd2UgIDxtcm93ZUBhcHBsZS5jb20+CiAKICAgICAgICAgVHJ5IGFuZCBmaXggdGhlIFF0IGJ1 aWxkLgpJbmRleDogV2ViQ29yZS9kb20vWE1MVG9rZW5pemVyTGlieG1sMi5jcHAKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gV2ViQ29yZS9kb20vWE1MVG9rZW5pemVyTGlieG1sMi5jcHAJKHJldmlzaW9uIDQyMDMy KQorKysgV2ViQ29yZS9kb20vWE1MVG9rZW5pemVyTGlieG1sMi5jcHAJKHdvcmtpbmcgY29weSkK QEAgLTEyOTUsNyArMTI5NSw4IEBAIGJvb2wgcGFyc2VYTUxEb2N1bWVudEZyYWdtZW50KGNvbnN0 IFN0cmkKIAogICAgIFhNTFRva2VuaXplciB0b2tlbml6ZXIoZnJhZ21lbnQsIHBhcmVudCk7CiAg ICAgCi0gICAgdG9rZW5pemVyLmluaXRpYWxpemVQYXJzZXJDb250ZXh0KGNodW5rLnV0ZjgoKS5k YXRhKCkpOworICAgIENTdHJpbmcgY2h1bmtBc1V0ZjggPSBjaHVuay51dGY4KCk7CisgICAgdG9r ZW5pemVyLmluaXRpYWxpemVQYXJzZXJDb250ZXh0KGNodW5rQXNVdGY4LmRhdGEoKSk7CiAKICAg ICB4bWxQYXJzZUNvbnRlbnQodG9rZW5pemVyLm1fY29udGV4dCk7CiAKQEAgLTEzMDMsNyArMTMw NCw3IEBAIGJvb2wgcGFyc2VYTUxEb2N1bWVudEZyYWdtZW50KGNvbnN0IFN0cmkKIAogICAgIC8v IENoZWNrIGlmIGFsbCB0aGUgY2h1bmsgaGFzIGJlZW4gcHJvY2Vzc2VkLgogICAgIGxvbmcgYnl0 ZXNQcm9jZXNzZWQgPSB4bWxCeXRlQ29uc3VtZWQodG9rZW5pemVyLm1fY29udGV4dCk7Ci0gICAg aWYgKGJ5dGVzUHJvY2Vzc2VkID09IC0xIHx8ICgodW5zaWduZWQgbG9uZylieXRlc1Byb2Nlc3Nl ZCkgPT0gc2l6ZW9mKFVDaGFyKSAqIGNodW5rLmxlbmd0aCgpKQorICAgIGlmIChieXRlc1Byb2Nl c3NlZCA9PSAtMSB8fCAoKHVuc2lnbmVkIGxvbmcpYnl0ZXNQcm9jZXNzZWQpICE9IGNodW5rQXNV dGY4Lmxlbmd0aCgpKQogICAgICAgICByZXR1cm4gZmFsc2U7CiAKICAgICAvLyBObyBlcnJvciBp ZiB0aGUgY2h1bmsgaXMgd2VsbCBmb3JtZWQgb3IgaXQgaXMgbm90IGJ1dCB3ZSBoYXZlIG5vIGVy cm9yLgpJbmRleDogTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRl c3RzL0NoYW5nZUxvZwkocmV2aXNpb24gNDIwMzIpCisrKyBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJ KHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwxMCBAQAorMjAwOS0wMy0yNyAgS2FpICA8c2V0IEVN QUlMX0FERFJFU1MgZW52aXJvbm1lbnQgdmFyaWFibGU+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBmYXN0L2lubmVySFRNTC9pbm5lckhUTUwtbmJz cC1leHBlY3RlZC50eHQ6IEFkZGVkLgorICAgICAgICAqIGZhc3QvaW5uZXJIVE1ML2lubmVySFRN TC1uYnNwLnhodG1sOiBBZGRlZC4KKwogMjAwOS0wMy0yNiAgRXJpYyBTZWlkZWwgIDxlcmljQHdl YmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGF2aWQgSHlhdHQuCkluZGV4OiBMYXlv dXRUZXN0cy9mYXN0L2lubmVySFRNTC9pbm5lckhUTUwtbmJzcC1leHBlY3RlZC50eHQKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gTGF5b3V0VGVzdHMvZmFzdC9pbm5lckhUTUwvaW5uZXJIVE1MLW5ic3AtZXhwZWN0 ZWQudHh0CShyZXZpc2lvbiAwKQorKysgTGF5b3V0VGVzdHMvZmFzdC9pbm5lckhUTUwvaW5uZXJI VE1MLW5ic3AtZXhwZWN0ZWQudHh0CShyZXZpc2lvbiAwKQpAQCAtMCwwICsxLDMgQEAKK1RoaXMg dGVzdCBjaGVja3MgdG8gc2VlIGlmIHNldHRpbmcgaW5uZXJIVE1MIHRvIGEgc2luZ2xlIG5vbi1i cmVha2luZyBzcGFjZSB3b3Jrcy4KK09yaWdpbmFsOiDCoAorUmVzdWx0OiAgIMKgCgpQcm9wZXJ0 eSBjaGFuZ2VzIG9uOiBMYXlvdXRUZXN0cy9mYXN0L2lubmVySFRNTC9pbm5lckhUTUwtbmJzcC1l eHBlY3RlZC50eHQKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwpBZGRlZDogc3ZuOmVvbC1zdHlsZQogICArIG5hdGl2ZQoK SW5kZXg6IExheW91dFRlc3RzL2Zhc3QvaW5uZXJIVE1ML2lubmVySFRNTC1uYnNwLnhodG1sCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIExheW91dFRlc3RzL2Zhc3QvaW5uZXJIVE1ML2lubmVySFRNTC1uYnNwLnho dG1sCShyZXZpc2lvbiAwKQorKysgTGF5b3V0VGVzdHMvZmFzdC9pbm5lckhUTUwvaW5uZXJIVE1M LW5ic3AueGh0bWwJKHJldmlzaW9uIDApCkBAIC0wLDAgKzEsMzUgQEAKKzw/eG1sIHZlcnNpb249 IjEuMCIgZW5jb2Rpbmc9IklTTy04ODU5LTEiPz4KKzwhRE9DVFlQRSBodG1sIFBVQkxJQyAiLS8v VzNDLy9EVEQgWEhUTUwgMS4wIFRyYW5zaXRpb25hbC8vRU4iICJodHRwOi8vd3d3LnczLm9yZy9U Ui94aHRtbDEvRFREL3hodG1sMS10cmFuc2l0aW9uYWwuZHRkIj4KKzxodG1sIHhtbG5zPSJodHRw Oi8vd3d3LnczLm9yZy8xOTk5L3hodG1sIj4KKzxoZWFkPjwvaGVhZD4KKzxib2R5PgorPGRpdiBp ZD0iZGVzY3JpcHRpb24iPlRoaXMgdGVzdCBjaGVja3MgdG8gc2VlIGlmIHNldHRpbmcgaW5uZXJI VE1MIHRvIGEgc2luZ2xlIG5vbi1icmVha2luZyBzcGFjZSB3b3Jrcy48L2Rpdj4KKworPGRpdiBp ZD0idGVzdCIgc3R5bGU9ImRpc3BsYXk6bm9uZSI+PC9kaXY+CisKKzxkaXYgaWQ9ImNvbnNvbGUi PjwvZGl2PgorCis8c2NyaXB0IGxhbmd1YWdlPSJqYXZhc2NyaXB0Ij4KK2lmICh3aW5kb3cubGF5 b3V0VGVzdENvbnRyb2xsZXIpCisgICAgd2luZG93LmxheW91dFRlc3RDb250cm9sbGVyLmR1bXBB c1RleHQoKTsKKworZnVuY3Rpb24gbG9nKG1lc3NhZ2UpIHsKKyAgICB2YXIgY29uc29sZSA9IGRv Y3VtZW50LmdldEVsZW1lbnRCeUlkKCJjb25zb2xlIik7CisgICAgdmFyIGxpID0gZG9jdW1lbnQu Y3JlYXRlRWxlbWVudCgibGkiKTsKKyAgICB2YXIgcHJlID0gZG9jdW1lbnQuY3JlYXRlRWxlbWVu dCgicHJlIik7CisgICAgcHJlLmFwcGVuZENoaWxkKGRvY3VtZW50LmNyZWF0ZVRleHROb2RlKG1l c3NhZ2UpKTsKKyAgICBsaS5hcHBlbmRDaGlsZChwcmUpOworICAgIGNvbnNvbGUuYXBwZW5kQ2hp bGQobGkpOworfQorCit2YXIgaHRtbCA9ICJcdTAwQTAiOworCit2YXIgZSA9IGRvY3VtZW50Lmdl dEVsZW1lbnRCeUlkKCJ0ZXN0Iik7CitlLmlubmVySFRNTCA9IGh0bWw7CisKK2xvZygiT3JpZ2lu YWw6ICIgKyBodG1sKTsKK2xvZygiUmVzdWx0OiAgICIgKyBlLmlubmVySFRNTCk7CisKKzwvc2Ny aXB0PgorPC9ib2R5PgorPC9odG1sPgoKUHJvcGVydHkgY2hhbmdlcyBvbjogTGF5b3V0VGVzdHMv ZmFzdC9pbm5lckhUTUwvaW5uZXJIVE1MLW5ic3AueGh0bWwKX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpBZGRlZDogc3Zu OmVvbC1zdHlsZQogICArIG5hdGl2ZQoK