Insert the error message block when stopping parsing and an error occurred

Insert the error message block when stopping parsing and an error occurred

When XMLDocumentParser::stopParsing() is called to stop parsing, call
insertErrorMessageBlock() to insert the <parsererror> element if an error
occurred.

The <parsererror> element was not being added to the document in certain
cases involving attribute parsing. For example, if an undeclared namespace
prefix was used in the document, then parsing would stop, but a <parsererror>
element would not be inserted into the document to indicate that a parsing
error occurred.

BUG=295809

[dtrebbien, 2014-11-30 21:20:23]

dtrebbien@gmail.com changed reviewers:
+ eae@chromium.org, ed@opera.com, eseidel@chromium.org, fs@opera.com,
leviw@chromium.org, pdr@chromium.org

[dtrebbien, 2014-11-30 21:23:04]

As I can't run all of the layout tests for myself on Yosemite yet
(http://crbug.com/425571), can a couple of try bots be added to test out this
CL?

[leviw_travelin_and_unemployed, 2014-12-01 02:04:12]

I added a bunch of trybots.

[dtrebbien, 2014-12-04 12:43:49]

On 2014/12/01 02:04:12, leviw wrote:
> I added a bunch of trybots.

Thanks.

The trybot results look good.  I just uploaded a small update to the test case
following a suggestion by fs on a different CL
(https://codereview.chromium.org/745383007/ ).

[Erik Dahlström (inactive), 2015-01-14 13:35:41]

Showing the parsing error sounds good to me.

https://codereview.chromium.org/771493002/diff/20001/Source/core/xml/parser/X...
File Source/core/xml/parser/XMLDocumentParser.cpp (left):

https://codereview.chromium.org/771493002/diff/20001/Source/core/xml/parser/X...
Source/core/xml/parser/XMLDocumentParser.cpp:426: insertErrorMessageBlock();
Under which circumstances would the parser be stopped when we reach here?

https://codereview.chromium.org/771493002/diff/20001/Source/core/xml/parser/X...
File Source/core/xml/parser/XMLDocumentParser.cpp (right):

https://codereview.chromium.org/771493002/diff/20001/Source/core/xml/parser/X...
Source/core/xml/parser/XMLDocumentParser.cpp:1537: if (!m_parsingFragment &&
m_sawError && !isStopped())
What about (m_parsingFragment && m_sawError && !isStopped())? How is that
handled wrt parsing errors?

[dtrebbien, 2015-01-20 00:40:59]

On 2015/01/14 13:35:41, Erik Dahlström wrote:
> Showing the parsing error sounds good to me.
> 
>
https://codereview.chromium.org/771493002/diff/20001/Source/core/xml/parser/X...
> File Source/core/xml/parser/XMLDocumentParser.cpp (left):
> 
>
https://codereview.chromium.org/771493002/diff/20001/Source/core/xml/parser/X...
> Source/core/xml/parser/XMLDocumentParser.cpp:426: insertErrorMessageBlock();
> Under which circumstances would the parser be stopped when we reach here?
> 
>
https://codereview.chromium.org/771493002/diff/20001/Source/core/xml/parser/X...
> File Source/core/xml/parser/XMLDocumentParser.cpp (right):
> 
>
https://codereview.chromium.org/771493002/diff/20001/Source/core/xml/parser/X...
> Source/core/xml/parser/XMLDocumentParser.cpp:1537: if (!m_parsingFragment &&
> m_sawError && !isStopped())
> What about (m_parsingFragment && m_sawError && !isStopped())? How is that
> handled wrt parsing errors?

Hi Erik,

Thanks for the feedback.

I have been trying commenting out the modifications individually to remind
myself why I added them.  I think that something has changed, though.  For
example, I know that I needed `!m_parsingFragment' or else I would see four
extra <parsererror> blocks when running
LayoutTests/fast/dom/xhtml-fragment-parsing-exceptions.xhtml, but it appears
that this is no longer necessary for some reason.

It may be that the patch can be simplified.  I am looking into this...

[dtrebbien, 2015-01-21 00:11:36]

I uploaded a new patch that only modifies XMLDocumentParser::stopParsing().  The
CL description was updated as well.

[Erik Dahlström (inactive), 2015-04-28 15:18:34]

On 2015/01/21 00:11:36, dtrebbien wrote:
> I uploaded a new patch that only modifies XMLDocumentParser::stopParsing(). 
The
> CL description was updated as well.

Patchset #3 seems to add two error messages for this testcase:
http://src.chromium.org/viewvc/blink/trunk/LayoutTests/svg/custom/missing-xli....

[dtrebbien, 2015-05-03 12:16:43]

On 2015/04/28 15:18:34, Erik Dahlström wrote:
> On 2015/01/21 00:11:36, dtrebbien wrote:
> > I uploaded a new patch that only modifies XMLDocumentParser::stopParsing(). 
> The
> > CL description was updated as well.
> 
> Patchset #3 seems to add two error messages for this testcase:
>
http://src.chromium.org/viewvc/blink/trunk/LayoutTests/svg/custom/missing-xli....

I have uploaded a new Patchset to fix this regression (I added back `&&
!isStopped()' in XMLDocumentParser::end()).