[webkit-reviews] review denied: [Bug 87578] [FileAPI] The result attribute of FileReader shuold use null to replace empty string : [Attachment 144210] Patch

[bugzilla-daemon at webkit.org]

Jian Li <jianli at chromium.org> has denied  review:
Bug 87578: [FileAPI] The result attribute of FileReader shuold use null to
replace empty string
https://bugs.webkit.org/show_bug.cgi?id=87578

Attachment 144210: Patch
https://bugs.webkit.org/attachment.cgi?id=144210&action=review

------- Additional Comments from Jian Li <jianli at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=144210&action=review


> Source/WebCore/fileapi/FileReader.cpp:248
> +    if (ret.isEmpty())

This would cause NULL being returned when reading an empty file. I think you
should change it to:
    if (m_loader && m_loader->stringResult())
	return m_loader->stringResult();
    return String();

> LayoutTests/fast/files/blob-slice-test-expected.txt:3
> +Slicing from 2 to 2: null

The spec does not say that null needs to be returned for empty file/blob. I
think we should still return empty string for this case.

You could add test cases to cover the scenarios that result is being read when
readyState is EMPTY or an error occurs.