I get the following compile-time errors when building WebKit with WML support enabled: /Users/dbates/Desktop/WebKit/WebCore/wml/WMLCardElement.cpp: In static member function 'static WebCore::WMLCardElement* WebCore::WMLCardElement::findNamedCardInDocument(WebCore::Document*, const WebCore::String&)': /Users/dbates/Desktop/WebKit/WebCore/wml/WMLCardElement.cpp:290: error: 'class WebCore::WMLCardElement' has no member named 'getIDAttribute' /Users/dbates/Desktop/WebKit/WebCore/wml/WMLDocument.cpp: In constructor 'WebCore::WMLDocument::WMLDocument(WebCore::Frame*)': /Users/dbates/Desktop/WebKit/WebCore/wml/WMLDocument.cpp:39: error: no matching function for call to 'WebCore::Document::Document(WebCore::Frame*&, bool, bool)' /Users/dbates/Desktop/WebKit/WebCore/dom/Document.h:1009: note: candidates are: WebCore::Document::Document(WebCore::Frame*, const WebCore::KURL&, bool, bool) /Users/dbates/Desktop/WebKit/WebCore/dom/Document.h:189: note: WebCore::Document::Document(const WebCore::Document&) /Users/dbates/Desktop/WebKit/WebCore/wml/WMLDocument.cpp: In member function 'virtual void WebCore::WMLDocument::finishedParsing()': /Users/dbates/Desktop/WebKit/WebCore/wml/WMLDocument.cpp:51: error: 'class WebCore::DocumentParser' has no member named 'wellFormed' The following errors are related to Bug #41186: /Users/dbates/Desktop/WebKit/WebCore/dom/XMLDocumentParser.h: In function 'void WebCore::reportWMLError(WebCore::Document*, WebCore::WMLErrorCode)': /Users/dbates/Desktop/WebKit/WebCore/dom/XMLDocumentParser.h:103: error: 'virtual bool WebCore::XMLDocumentParser::wellFormed() const' is private /Users/dbates/Desktop/WebKit/WebCore/wml/WMLErrorHandling.cpp:48: error: within this context /Users/dbates/Desktop/WebKit/WebCore/dom/XMLDocumentParser.h:104: error: 'virtual int WebCore::XMLDocumentParser::lineNumber() const' is private /Users/dbates/Desktop/WebKit/WebCore/wml/WMLErrorHandling.cpp:51: error: within this context /Users/dbates/Desktop/WebKit/WebCore/dom/XMLDocumentParser.h:105: error: 'virtual int WebCore::XMLDocumentParser::columnNumber() const' is private /Users/dbates/Desktop/WebKit/WebCore/wml/WMLErrorHandling.cpp:51: error: within this context /Users/dbates/Desktop/WebKit/WebCore/wml/WMLGoElement.cpp: In member function 'void WebCore::WMLGoElement::preparePOSTRequest(WebCore::ResourceRequest&, bool, const WebCore::String&)': /Users/dbates/Desktop/WebKit/WebCore/wml/WMLGoElement.cpp:158: error: 'FormDataBuilder' has not been declared /Users/dbates/Desktop/WebKit/WebCore/wml/WMLGoElement.cpp: In member function 'WTF::PassRefPtr<WebCore::FormData> WebCore::WMLGoElement::createFormData(const WTF::CString&)': /Users/dbates/Desktop/WebKit/WebCore/wml/WMLGoElement.cpp:191: error: 'FormDataBuilder' has not been declared /Users/dbates/Desktop/WebKit/WebCore/wml/WMLGoElement.cpp:202: error: 'FormDataBuilder' has not been declared /Users/dbates/Desktop/WebKit/WebCore/wml/WMLGoElement.cpp:203: error: 'FormDataBuilder' has not been declared /Users/dbates/Desktop/WebKit/WebCore/wml/WMLGoElement.cpp:211: error: 'FormDataBuilder' has not been declared /Users/dbates/Desktop/WebKit/WebCore/wml/WMLGoElement.cpp:215: error: 'FormDataBuilder' has not been declared /Users/dbates/Desktop/WebKit/WebCore/wml/WMLInputElement.cpp:140: warning: unused parameter 'sendChangeEvent' /Users/dbates/Desktop/WebKit/WebCore/wml/WMLInputElement.cpp:159: warning: unused parameter 'value'
Now, it still failed to compile with WML as follows, CXX WebCore/dom/libwebkitgtk_1_0_la-Document.lo CXX WebCore/dom/libwebkitgtk_1_0_la-DocumentFragment.lo CXX WebCore/dom/libwebkitgtk_1_0_la-ProcessingInstruction.lo CXX WebCore/dom/libwebkitgtk_1_0_la-XMLDocumentParser.lo CXX WebCore/dom/libwebkitgtk_1_0_la-XMLDocumentParserLibxml2.lo CXX WebCore/loader/libwebkitgtk_1_0_la-FrameLoader.lo CXX WebCore/svg/libwebkitgtk_1_0_la-SVGUseElement.lo CXX WebCore/wml/libwebkitgtk_1_0_la-WMLErrorHandling.lo ../../WebCore/dom/XMLDocumentParser.h: In function ‘void WebCore::reportWMLError(WebCore::Document*, WebCore::WMLErrorCode)’: ../../WebCore/dom/XMLDocumentParser.h:132: error: ‘int WebCore::XMLDocumentParser::columnNumber() const’ is private ../../WebCore/wml/WMLErrorHandling.cpp:51: error: within this context make[1]: *** [WebCore/wml/libwebkitgtk_1_0_la-WMLErrorHandling.lo] Error 1
Created attachment 75915 [details] Proposed Patch It fixes this build error.
Comment on attachment 75915 [details] Proposed Patch OK.
Comment on attachment 75915 [details] Proposed Patch Actually... Why is this needed?
TextPosition is the new way to do this: http://trac.webkit.org/browser/trunk/WebCore/dom/ScriptableDocumentParser.h
(In reply to comment #5) > TextPosition is the new way to do this: > http://trac.webkit.org/browser/trunk/WebCore/dom/ScriptableDocumentParser.h It doesn't fix this problem.
Can you elaborate?
Created attachment 76335 [details] Proposed Patch2 I'm not sure if I use TextPosition in a right way.
Comment on attachment 76335 [details] Proposed Patch2 I'm confused why you're moving lineNumber and columnNumber? Neither of these need to be public anymore. textPosition is the new way to do this all I believe.
Peter added the TextPosition stuff.
(In reply to comment #10) > Peter added the TextPosition stuff. The idea behind TextPosition is to keep line/column pair together and what's more important to take care of 0-based/1-based numbers. Incidentally, such issue seems to be hidden in "Patch2": actual arguments of handleError are explicitly 0-based, while this method treats them as 1-based numbers (in user messages all numbers are expected to be 1-based I believe). I propose we adding TextPosition to handleError signature. Let me prepare a patch about this real soon.
Created attachment 76395 [details] extending handleError signature -- not a regular patch I propose this additional patch -- switches handleError method to TextPosition1. This patch does not fix the bug itself, only helps. Joone, could you please try if it works for you? If it works, you can merge it with your change. I guess you will also need to convert your TextPosition0 (zero-based value) to TextPosition1 (one-based value expected in handleError), WTF::toOneBasedTextPosition can be used for this. Peter
(In reply to comment #9) > (From update of attachment 76335 [details]) > I'm confused why you're moving lineNumber and columnNumber? Neither of these need to be public anymore. textPosition is the new way to do this all I believe. Yes, I know. I made them private as follows, private: XMLDocumentParser(Document*, FrameView* = 0); XMLDocumentParser(DocumentFragment*, Element*, FragmentScriptingPermission); // From DocumentParser virtual void insert(const SegmentedString&); virtual void append(const SegmentedString&); virtual void finish(); virtual bool finishWasCalled(); virtual bool isWaitingForScripts() const; virtual void stopParsing(); virtual void detach(); virtual int lineNumber() const; int columnNumber() const;
Comment on attachment 76335 [details] Proposed Patch2 This seems OK. Peter's fix is definitely better, and it would be good to follow up with a patch which does peter's proposed refactoring.
The commit-queue encountered the following flaky tests while processing attachment 76335 [details]: inspector/elements-panel-styles.html bug 50987 (author: pfeldman@chromium.org) The commit-queue is continuing to process your patch.
Comment on attachment 76335 [details] Proposed Patch2 Clearing flags on attachment: 76335 Committed r73975: <http://trac.webkit.org/changeset/73975>
All reviewed patches have been landed. Closing bug.
(In reply to comment #12) > Created an attachment (id=76395) [details] > extending handleError signature -- not a regular patch > > I propose this additional patch -- switches handleError method to TextPosition1. This patch does not fix the bug itself, only helps. > > Joone, could you please try if it works for you? If it works, you can merge it with your change. I guess you will also need to convert your TextPosition0 (zero-based value) to TextPosition1 (one-based value expected in handleError), WTF::toOneBasedTextPosition can be used for this. > > Peter I tried to use TextPosition1 as follows, parser->handleError(XMLDocumentParser::fatal, errorMessage.latin1().data(), toOneBasedTextPosition(parser->textPosition()).m_line.oneBasedInt(), toOneBasedTextPosition(parser->textPosition()).m_column.oneBasedInt()); It gives us wrong error report. For example, 16 <card id="card2" title="Answer"> *17 <p>You selected: $($$name)</p> 18 </card> 19 </wml 17 line has an error, so if we use TextPosition1, it reports the following error, "This page contains the following errors: error on line 18 at column 36: A variable reference uses invalid syntax. Below is a rendering of the page up to the first error." I guess that the error should be on line 17 at column 26. If we use TextPosition0, it reports, "This page contains the following errors: error on line 17 at column 35: A variable reference uses invalid syntax. Below is a rendering of the page up to the first error." It seems right except the column number, but actually, is line 16 right if we use TextPosition0?
Oh, I see. I gets a little more complicated that I thought. I remember I didn't quite complete with refactoring in XML parser because I noticed an inconsistency around lineNumber/columnNumber methods: in some cases their result was used as 0-based, in other cases it was clearly used as 1-based. The problem was I didn't know which is correct. So I put TextPosition there, but I retained inconsistency by adding an explicit casting code. What I did was the following: XMLDocumentParser::textPosition method expects lineNumber/columnNumber works as a wrapper for lineNumber/columnNumber methods. Since most of the code expected lineNumber/columnNumber to return 0-based values, textPosition method takes them as 0-based. Now I remember I had to add a helper method textPositionOneBased(). It returns same numbers, but treats them as 1-based; probably it should work for you. Also it would be of a great help if you could tell what XMLDocumentParser::lineNumber/XMLDocumentParser::columnNumber result actually is: 0-based or 1-based. Please add a comment in this case. However it's a bit complicated by the fact there are 2 different implementations of the parser: "XMLDocumentParserQT.cpp" and "XMLDocumentParserLibxml2.cpp" Adding annotation to at least one of these files will help fixing it in the future. > I tried to use TextPosition1 as follows, > > parser->handleError(XMLDocumentParser::fatal, errorMessage.latin1().data(), toOneBasedTextPosition(parser->textPosition()).m_line.oneBasedInt(), toOneBasedTextPosition(parser->textPosition()).m_column.oneBasedInt()); > > It gives us wrong error report. > > For example, > > 16 <card id="card2" title="Answer"> > *17 <p>You selected: $($$name)</p> > 18 </card> > 19 </wml > > 17 line has an error, so if we use TextPosition1, it reports the following error, > > "This page contains the following errors: > error on line 18 at column 36: A variable reference uses invalid syntax. > Below is a rendering of the page up to the first error." > > I guess that the error should be on line 17 at column 26. > > If we use TextPosition0, it reports, > > "This page contains the following errors: > error on line 17 at column 35: A variable reference uses invalid syntax. > Below is a rendering of the page up to the first error." > > It seems right except the column number, but actually, is line 16 right if we use TextPosition0? Column number looks quite weird, I have no idea about it. You are right, TextPosition0 should contain 16. This is because of inconsistency that I retained to keep refactoring accurate.
> What I did was the following: > XMLDocumentParser::textPosition method expects lineNumber/columnNumber works as a wrapper for lineNumber/columnNumber methods. Since most of the code expected lineNumber/columnNumber to return 0-based values, textPosition method takes them as 0-based. Sorry, I meant: "XMLDocumentParser::textPosition method works as a wrapper for lineNumber/columnNumber methods." And thank you for your investigation. Peter
(In reply to comment #19) > Also it would be of a great help if you could tell what XMLDocumentParser::lineNumber/XMLDocumentParser::columnNumber result actually is: 0-based or 1-based. Please add a comment in this case. However it's a bit complicated by the fact there are 2 different implementations of the parser: "XMLDocumentParserQT.cpp" and "XMLDocumentParserLibxml2.cpp" > > Adding annotation to at least one of these files will help fixing it in the future. XMLDocumentParser uses 1-based values, so I filed a new bug https://bugs.webkit.org/show_bug.cgi?id=5160
(In reply to comment #21) > (In reply to comment #19) > > > Also it would be of a great help if you could tell what XMLDocumentParser::lineNumber/XMLDocumentParser::columnNumber result actually is: 0-based or 1-based. Please add a comment in this case. However it's a bit complicated by the fact there are 2 different implementations of the parser: "XMLDocumentParserQT.cpp" and "XMLDocumentParserLibxml2.cpp" > > > > Adding annotation to at least one of these files will help fixing it in the future. > > XMLDocumentParser uses 1-based values, so I filed a new bug https://bugs.webkit.org/show_bug.cgi?id=5160 https://bugs.webkit.org/show_bug.cgi?id=51601
>> complicated by the fact there are 2 different implementations of the parser: >> "XMLDocumentParserQT.cpp" and "XMLDocumentParserLibxml2.cpp" > XMLDocumentParser uses 1-based values, so I filed a new bug https://bugs.webkit.org/show_bug.cgi?id=5160 To me your patch is fine. Thank you for checking with XML parser; however, to be accurate I need to know, whether it was "Libxml2" or "QT" version. (The brutal way to check would be to spoil both "XMLDocumentParserQT.cpp" and "XMLDocumentParserLibxml2.cpp" and see which one emits compile error in your build configuration).