Bug 43180 – Change permission to access methods in XMLDocumentParser.h

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

[Gyuyoung Kim]

When WML is enabled, there are build breaks by 3 methods of XMLDocumentParser.h.
The methods are wellFormed(), lineNumber() and columnNumber().

These methods is private accessibility. However, WMLErrorHandling.cpp file is using the methods.
I am not sure if this accessibility change is correct or not. If we can't change the accessibility, 
We need to modify the WMLErrorHandling.cpp file.

[Gyuyoung Kim]

Created attachment 62928 [details]
Patch

[Darin Adler]

Comment on attachment 62928 [details]
Patch

This is not the right fix. If we have to make these public we will, but we should not use conditionals.

Where is the code in WML that uses these? Why is it using an XMLDocumentParser* instead of a DocumentParser*?

[Gyuyoung Kim]

(In reply to comment #2)
> (From update of attachment 62928 [details])
> This is not the right fix. If we have to make these public we will, but we should not use conditionals.
> 
> Where is the code in WML that uses these? Why is it using an XMLDocumentParser* instead of a DocumentParser*?

The reportWMLError() of WMLErrorHandling.cpp is using these functions. It seems that the accessibility was changed by Bug 41114 - Start to clean up DocumentParser interface. But, there are no the reasons in the bug.

Frankly, I don't know why this WMLErrorHandling.cpp uses XMLDocumentParser.cpp yet. I am trying to trace the reason via log.

I suspects that WML guys uses XMLDocumentParser to handle error via these functions.

In my opinion, there are two solutions. One is we change these functions's accessibility with public. As you said, other is that we don't use XMLDocumentParser in WMLErrorHandling.cpp.

[Darin Adler]

(In reply to comment #3)
> In my opinion, there are two solutions. One is we change these functions's accessibility with public. As you said, other is that we don't use XMLDocumentParser in WMLErrorHandling.cpp.

I think making them public is fine.

[Gyuyoung Kim]

Created attachment 63523 [details]
Patch

Sorry for my late response. I was summer vacation.
Ok, I change the accessibility with public. Please review this patch. :)

[Darin Adler]

Comment on attachment 63523 [details]
Patch

> +        // WMLErrorHandling uses these methods.

In future, please use the C++ term, functions, rather than calling these methods.

[Gyuyoung Kim]

(In reply to comment #6)
> (From update of attachment 63523 [details])
> > +        // WMLErrorHandling uses these methods.
> 
> In future, please use the C++ term, functions, rather than calling these methods.

Ok, I keep in mind. Thank you.

[Gyuyoung Kim]

Hello darin,

It seems that reviewed patch doesn't pass style error for gtk. However, I don't know why this patch cannot pass the gtk style error. If there are no style errors, could you commit this patch manually ?

But, if I need to modify this patch, please let me know. Thank you.

[Gyuyoung Kim]

Created attachment 63676 [details]
Patch for commit

Hello Darin,

I change "methods' with "functions" for commit. :)

[Darin Adler]

(In reply to comment #8)
> It seems that reviewed patch doesn't pass style error for gtk.

That’s incorrect. I just reviewed the patch before the GTK EWS bot got a chance to compile it, and once reviewed the bots stop processing a path. There is nothing wrong with the patch.

[Darin Adler]

Comment on attachment 63523 [details]
Patch

Clear the flags on the older patch.

[WebKit Commit Bot]

Comment on attachment 63676 [details]
Patch for commit

Clearing flags on attachment: 63676

Committed r64855: <http://trac.webkit.org/changeset/64855>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.