Bug 24883 – Bad success test in parseXMLDocumentFragment in XMLTokenizerLibxml2.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Kai Brüning]

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).

[Kai Brüning]

Created attachment 29010 [details]
Fix with test case

[Eric Seidel (no email)]

Comment on attachment 29010 [details]
Fix with test case

This looks correct to me.  Thanks for the patch and layout test!

[Eric Seidel (no email)]

Comment on attachment 29010 [details]
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!

[Eric Seidel (no email)]

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