Expose the editing behavior setting to DRT, as discussed in https://lists.webkit.org/pipermail/webkit-dev/2010-March/012118.html It is a follow of of bug 38603, witch missed the Qt bits. Patch coming ...
Created attachment 57034 [details] (committed with r60189, reviewed by Ojan Vafai) patch v1
Comment on attachment 57034 [details] (committed with r60189, reviewed by Ojan Vafai) patch v1 (From update of attachment 57034 [details]) Clearing flags on attachment: 57034 Committed r60189: <http://trac.webkit.org/changeset/60189>
This patch introduced the following warning: webkit/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp: In static member function âstatic void DumpRenderTreeSupportQt::setEditingBehavior(QWebPage*, const QString&): webkit/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:479: warning: "coreEditingBehavior" may be used uninitialized in this function
Created attachment 57403 [details] proposed patch
(In reply to comment #3) > This patch introduced the following warning: > > webkit/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp: In static member function âstatic void DumpRenderTreeSupportQt::setEditingBehavior(QWebPage*, const QString&): > > webkit/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:479: warning: "coreEditingBehavior" may be used uninitialized in this function My bad. I saw that, and would patch it when I was on a clean tree, but good you came first :) else if (editingBehavior == "mac") coreEditingBehavior = EditingMacBehavior; + else + return; Personally I would add an ASSERT_NOT_REACHED() as well. This shouldn't get never executed at all! other than that, lgtm, thank you Laszlo.
Comment on attachment 57403 [details] proposed patch Should we log in this case?
Seems like programmer error to pass a non-supported value here as part of a test case.
Comment on attachment 57403 [details] proposed patch We could even ASSERT_NOT_REACHED instead of loging. That's probably better. Yeah this should just ASSERT_NOT_REACHED.
Created attachment 57572 [details] patch v2 - adding ASSERT_NOT_REACHED
Comment on attachment 57572 [details] patch v2 - adding ASSERT_NOT_REACHED When I add ASSERT_NOT_REACHED I often put a comment as to why it should not be reached. In this case: # You've hit this ASSERT either because the layout test is using an invalid string or because Qt's DumpRenderTree is missing support for that editing behavior. You've added an unconditional return due to missing {}
Created attachment 57973 [details] (committed with r60778, reviewed by Kenneth and Eric) patch v2.1 - adding ASSERT_NOT_REACHED given the long delay, I felt free to re-upload the patch adding ASSERT_NOT_REACHED() and enclosing "{...}" to the return statment.
Comment on attachment 57973 [details] (committed with r60778, reviewed by Kenneth and Eric) patch v2.1 - adding ASSERT_NOT_REACHED Clearing flags on attachment: 57973 Committed r60778: <http://trac.webkit.org/changeset/60778>