[webkit-reviews] review denied: [Bug 31144] [Qt] The XML tokenizer reports a parse error twice if it occurs before the document element is found. : [Attachment 42530] proposed patch

[bugzilla-daemon at webkit.org]

Holger Freyther <zecke at selfish.org> has denied Jakub Wieczorek
<faw217 at gmail.com>'s request for review:
Bug 31144: [Qt] The XML tokenizer reports a parse error twice if it occurs
before the document element is found.
https://bugs.webkit.org/show_bug.cgi?id=31144

Attachment 42530: proposed patch
https://bugs.webkit.org/attachment.cgi?id=42530&action=review

------- Additional Comments from Holger Freyther <zecke at selfish.org>

> -    if (m_stream.error() == QXmlStreamReader::PrematureEndOfDocumentError
> -	   || (m_wroteText && !m_sawFirstElement && !m_sawXSLTransform))
> +    if (m_stream.error() == QXmlStreamReader::PrematureEndOfDocumentError)

I'm not convinced. Why do you remove m_sawXSLTransform? What is with the
originally mentioned XML Http Request test, is it still passing, why it passing
without the change?

So the question is the following:
  "libxml2 has the semantic that when writing an empty string and finishing it,
it will report an error. For QXmlStreamReader this is valid."

Is that still true, was that true, was the semantic of QXmlStreamReader
changed?



> diff --git a/WebCore/manual-tests/qt/parse-error-reported-twice.xml
b/WebCore/manual-tests/qt/parse-error-reported-twice.xml
> new file mode 100644
> index 0000000..0b97956
> --- /dev/null
> +++ b/WebCore/manual-tests/qt/parse-error-reported-twice.xml
> @@ -0,0 +1,5 @@
> +<?xml version="1.0" encoding="utf-8"?>
> +<!-- Trigger a parse error in the doctype. -->
> +<!DOCTYPE doc {}>
> +<doc></doc>
> +<!-- The tokenizer should report a parse failure _once_. -->
> -- 
> 1.6.4.2
> 

Make that a proper layout test, don't worry about the locale it is the same for
everyone and we can change it via the LayoutTestController