Make DocumentParser API private on subclasses to catch misuse bugs
Created attachment 59715 [details] Patch
Created attachment 59731 [details] Got rid of reference hackery
Comment on attachment 59731 [details] Got rid of reference hackery Much better. Thank you. :)
Committed r61835: <http://trac.webkit.org/changeset/61835>
This change broke WML support by changing the visibility of the functions wellFormed(), lineNumber(), and columnNumber() from public to private. These functions are used in WMLErrorHandling::reportWMLError(). For instance, wellFormed() is called on line 48 <http://trac.webkit.org/browser/trunk/WebCore/wml/WMLErrorHandling.cpp?rev=61104#L48>. I'm not familiar with this area of the code. Is there an alternative way to get this information? Should we use the NVI idiom here? Or should we re-expose these as public? CC'ing Nikolas Zimmermann since he worked on the WML code.
(In reply to comment #5) > This change broke WML support by changing the visibility of the functions wellFormed(), lineNumber(), and columnNumber() from public to private. These functions are used in WMLErrorHandling::reportWMLError(). For instance, wellFormed() is called on line 48 <http://trac.webkit.org/browser/trunk/WebCore/wml/WMLErrorHandling.cpp?rev=61104#L48>. > > I'm not familiar with this area of the code. Is there an alternative way to get this information? Should we use the NVI idiom here? Or should we re-expose these as public? > > CC'ing Nikolas Zimmermann since he worked on the WML code. I should be more precise. This change contributed to breaking WML support. WML support is broken in other places as well.
Comment on attachment 59731 [details] Got rid of reference hackery Rejecting patch 59731 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Adam Barth', u'--force']" exit_code: 1 Last 500 characters of output: LegacyHTMLTreeBuilder.cpp Hunk #1 FAILED at 1648. Hunk #2 FAILED at 1672. 2 out of 2 hunks FAILED -- saving rejects to file WebCore/html/LegacyHTMLTreeBuilder.cpp.rej patching file WebCore/loader/DocumentWriter.cpp Hunk #1 FAILED at 72. 1 out of 1 hunk FAILED -- saving rejects to file WebCore/loader/DocumentWriter.cpp.rej patching file WebCore/loader/TextDocument.cpp Hunk #1 FAILED at 44. Hunk #2 FAILED at 61. 2 out of 2 hunks FAILED -- saving rejects to file WebCore/loader/TextDocument.cpp.rej Full output: http://queues.webkit.org/results/3617103
(In reply to comment #5) > This change broke WML support by changing the visibility of the functions wellFormed(), lineNumber(), and columnNumber() from public to private. These functions are used in WMLErrorHandling::reportWMLError(). For instance, wellFormed() is called on line 48 <http://trac.webkit.org/browser/trunk/WebCore/wml/WMLErrorHandling.cpp?rev=61104#L48>. That's incorrect. wellFormed, lineNumber and columnNumber no longer exist on DocumentParser, only on the subclass ScriptableDocumentParser. What does WML use for parsing? Its own DocumentParser subclass? If so, that should be a subclass of ScriptableDocumentParser. In any case, WML code should be dealing with more specific pointer types when possible. Please open a new bug. Also, WML is broken and really should be ripped out from the tree in its broken, unmaintained, poorly-written state. If WML ever wants to be a real boy, its going to need a build-bot, as well as some cleanup.
(In reply to comment #8) > (In reply to comment #5) > > This change broke WML support by changing the visibility of the functions wellFormed(), lineNumber(), and columnNumber() from public to private. These functions are used in WMLErrorHandling::reportWMLError(). For instance, wellFormed() is called on line 48 <http://trac.webkit.org/browser/trunk/WebCore/wml/WMLErrorHandling.cpp?rev=61104#L48>. > > That's incorrect. wellFormed, lineNumber and columnNumber no longer exist on DocumentParser, only on the subclass ScriptableDocumentParser. > > What does WML use for parsing? Its own DocumentParser subclass? If so, that should be a subclass of ScriptableDocumentParser. In any case, WML code should be dealing with more specific pointer types when possible. WML is not scriptable, that should be remembered. > > Please open a new bug. > > Also, WML is broken and really should be ripped out from the tree in its broken, unmaintained, poorly-written state. If WML ever wants to be a real boy, its going to need a build-bot, as well as some cleanup. WML is unmaintained at the moment, that's true, but it's _not_ poorly written. I rewrote the WML support from scratch, and don't think it contains any poor code, except the parsing bits.