| Summary: | [Qt] Add support for LayoutTestController commands. | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Robert Hogan <robert> | ||||||||||||||||||||||||||
| Component: | Tools / Tests | Assignee: | Robert Hogan <robert> | ||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||
| Severity: | Enhancement | CC: | abarth, commit-queue, eric, hausmann, jturcotte, kenneth, kent.hansen, luiz, ossy, vestbo, webkit.review.bot, zecke | ||||||||||||||||||||||||||
| Priority: | P2 | Keywords: | Qt | ||||||||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||
Created attachment 50171 [details]
Patch
My local results for:
editing/deleting/5300379.html
editing/deleting/smart-delete-003.html
editing/deleting/smart-delete-004.html
editing/selection/5195166-1.html
editing/selection/5195166-2.html
are slightly different to those in LayoutTests/platform/qt/*expected.txt. But presumably the buildbot has the right fonts installed and I don't, so I haven't updated them.
The patch is fine, for the API it might be better to have an enum for each command and then a generic isCommandEnabled, setCommandEnabled function? (In reply to comment #2) > The patch is fine, for the API it might be better to have an enum for each > command and then a generic isCommandEnabled, setCommandEnabled function? There isn't a setCommandEnabled function required by the LayoutTestController. Also, isCommandEnabled is only ever called by the DRT so not sure of value of enumerating every valid command since those are already enumerated by WebCore. Have a feeling I've missed your point though. (In reply to comment #3) > (In reply to comment #2) > > The patch is fine, for the API it might be better to have an enum for each > > command and then a generic isCommandEnabled, setCommandEnabled function? > > There isn't a setCommandEnabled function required by the LayoutTestController. > Also, isCommandEnabled is only ever called by the DRT so not sure of value of > enumerating every valid command since those are already enumerated by WebCore. > > Have a feeling I've missed your point though. The point is. You will be adding ~10-15 public API methods (setters and getters) that have little usage outside of the DRT. This raises my concern that this might be the wrong approach. Please follow the QtWebKit bug reporting guidelines when reporting bugs. See http://trac.webkit.org/wiki/QtWebKitBugs Specifically: - The 'QtWebKit' component should only be used for bugs/features in the public QtWebKit API layer, not to signify that the bug is specific to the Qt port of WebKit http://trac.webkit.org/wiki/QtWebKitBugs#Component - Add the keyword 'Qt' to signal that it's a Qt-related bug http://trac.webkit.org/wiki/QtWebKitBugs#Keywords >
> The point is. You will be adding ~10-15 public API methods (setters and
> getters) that have little usage outside of the DRT. This raises my concern that
> this might be the wrong approach.
Think we're talking at cross-purposes here. This is the isCommandEnabled API the patch adds to the DRT:
+bool LayoutTestController::isCommandEnabled(const QString& name) const
+{
+ return qt_drt_isCommandEnabled(m_drt->webPage(), name);
+}
+
+bool QWEBKIT_EXPORT qt_drt_isCommandEnabled(QWebPage* page, const QString& name)
+{
+ return page->handle()->page->focusController()->focusedOrMainFrame()->editor()->command(name).isEnabled();
+}
+
No need to add any enums surely? (Apologies if I'm completely missing your point).
Comment on attachment 50171 [details]
Patch
Looks good in general, yay for passing more tests.
The part I'm unhappy about are the functions added to QWebPage:
* We usually don't have toggle() functions for plain boolean properties.
* The added properties are lacking documentation!
* I'd like to see the names discussed/proposed on the mailing list first, for a wider review.
* The selectTrailingWhitespaceEnabled property is lacking a Q_PROPERTY declaration
An alternative to the public API would and a cleanup of all the _drt_ functions would be to collect all those ugly qt_drt_foo functions, put them into WebCoreSupport/DumpRenderTreeSupport.cpp/h, _export_ the entire class and access it from the DRT by including the private header file.
(In reply to comment #7) > > * We usually don't have toggle() functions for plain boolean properties. > * The added properties are lacking documentation! > * I'd like to see the names discussed/proposed on the mailing list first, for a > wider review. > * The selectTrailingWhitespaceEnabled property is lacking a Q_PROPERTY > declaration > I was wrong to add these as API, they are only used by EditorClientQt. I've changed it so that they are private values accessible by EditorClient and moved the set*() API into DRTSupport. > An alternative to the public API would and a cleanup of all the _drt_ functions > would be to collect all those ugly qt_drt_foo functions, put them into > WebCoreSupport/DumpRenderTreeSupport.cpp/h, _export_ the entire class and > access it from the DRT by including the private header file. I went ahead and did this, though I suspect it will have to be a separate patch as the patch itself now in excess of 1K lines. I've briefly tested it and it seems to be working fine, though I'd appreciate comments on the following before splitting/finalizing it for commit and finishing testing: - How does use of the value of QWebPagePrivate::drtRun work exactly? Even at the moment it's referenced in FrameLoaderClientQt.cpp and I don't see how that class knows which instance of qwebpage it's referencing (or why it doesn't matter). I know it's a static value for the class, but does that mean once it's set it's true for all instances? Bit of a C++ 101 question but there you are! (My lack of clarity here means I'm not sure if I'm setting the value correctly in my patch.) Created attachment 50896 [details]
Updated Patch - with QDRTSupport
Created attachment 50960 [details]
Updated Patch - with QDRTSupport
Attachment 50960 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Last 3072 characters of output:
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:261: QDRTSupport::garbageCollector_collect is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:266: QDRTSupport::garbageCollector_collectOnAlternateThread is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebframe.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebframe.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage_p.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/tests/qwebpage/tst_qwebpage.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebsecurityorigin.cpp"
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:20: #ifndef header guard has wrong style, please use: DumpRenderTreeSupport_h [build/header_guard] [5]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:40: dump_editing_callbacks is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:41: dump_frame_loader is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:42: dump_resource_load_callbacks is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:51: webpage_groupName is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:52: webpage_setGroupName is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:53: dump_set_accepts_editing is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:59: garbageCollector_collect is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:60: garbageCollector_collectOnAlternateThread is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:66: webinspector_executeScript is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:67: webinspector_show is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:68: webinspector_close is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.h"
Total errors found: 30 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
We need to get this in quickly before the patch gets stale and hard to maintain. Comment on attachment 50960 [details] Updated Patch - with QDRTSupport > diff --git a/WebKit/qt/ChangeLog b/WebKit/qt/ChangeLog > index 546ac34..e73082a 100644 > --- a/WebKit/qt/ChangeLog > +++ b/WebKit/qt/ChangeLog > @@ -1,3 +1,49 @@ > +2010-03-07 Robert Hogan <robert@webkit.org> > + > + Reviewed by NOBODY (OOPS!). > + > + [Qt] Add support for LayoutTestController commands. > + > + Add support for LayoutTestController commands: > + setSmartInsertDeleteEnabled > + setSelectTrailingWhitespaceEnabled > + execCommand > + isCommandEnabled > + > + Unskip tests: > + editing/deleting/smart-editing-disabled.html > + editing/execCommand/19089.html > + editing/execCommand/delete-image-in-anchor.html > + editing/execCommand/enabling-and-selection-2.html > + editing/selection/doubleclick-whitespace-crash.html > + editing/selection/doubleclick-whitespace-img-crash.html > + editing/selection/doubleclick-whitespace.html > + editing/selection/select-line.html > + editing/deleting/5300379.html > + editing/deleting/smart-delete-003.html > + editing/deleting/smart-delete-004.html > + editing/selection/5195166-1.html > + editing/selection/5195166-2.html > + > + https://bugs.webkit.org/show_bug.cgi?id=35844 > + > + * Api/qwebpage.cpp: > + (qt_drt_executeCoreCommandByName): > + (qt_drt_isCommandEnabled): > + (QWebPagePrivate::QWebPagePrivate): > + (QWebPage::setSmartInsertDeleteEnabled): > + (QWebPage::smartInsertDeleteEnabled): > + (QWebPage::toggleSmartInsertDeleteEnabled): > + (QWebPage::setSelectTrailingWhitespaceEnabled): > + (QWebPage::selectTrailingWhitespaceEnabled): > + * Api/qwebpage.h: > + * Api/qwebpage_p.h: > + * WebCoreSupport/EditorClientQt.cpp: > + (WebCore::EditorClientQt::smartInsertDeleteEnabled): > + (WebCore::EditorClientQt::toggleSmartInsertDelete): > + (WebCore::EditorClientQt::isSelectTrailingWhitespaceEnabled): > + * WebCoreSupport/EditorClientQt.h: > + It looks like the ChangeLog is missing the qt_drt_* refactoring. > 2010-03-17 Csaba Osztrogonác <ossy@webkit.org> > > Reviewed by Kenneth Rohde Christiansen. > diff --git a/WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp b/WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp > new file mode 100644 > index 0000000..ccf13a3 > --- /dev/null > +++ b/WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp > @@ -0,0 +1,357 @@ > +/* > + * Copyright (C) 2007 Apple Inc. All rights reserved. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY > + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR > + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE COMPUTER, INC. OR > + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, > + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, > + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR > + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY > + * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ This doesn't look like the right copyright / license. The code comes mostly from qweb* classes and you should add your name, too, I think. Otherwise this looks great! Tor Arne, what do you think about the qt_drt_* refactoring? Created attachment 51181 [details]
Updated Patch
Updated per comments (and rebased). Still unsure about QWebPagePrivate::drtRun. I think it would be best if Ossy could confirm he has a clean test run on this before landing, running the layout tests on my machine, even with a clean head, is a total bust.
Attachment 51181 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:23: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:26: Alphabetical sorting problem. [build/include_order] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:27: Alphabetical sorting problem. [build/include_order] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:29: Alphabetical sorting problem. [build/include_order] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:31: "Frame.h" already included at WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:28 [build/include] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:36: Alphabetical sorting problem. [build/include_order] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:37: Alphabetical sorting problem. [build/include_order] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:38: Alphabetical sorting problem. [build/include_order] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:39: Alphabetical sorting problem. [build/include_order] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:42: Alphabetical sorting problem. [build/include_order] [4]
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:47: Alphabetical sorting problem. [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebframe.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebframe.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage_p.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/tests/qwebpage/tst_qwebpage.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebsecurityorigin.cpp"
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.h:23: #ifndef header guard has wrong style, please use: DumpRenderTreeSupport_h [build/header_guard] [5]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.h"
Total errors found: 12 in 23 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 51184 [details]
Updated Patch per Stylebot
Attachment 51184 [details] did not pass style-queue:
Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp:47: Alphabetical sorting problem. [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebframe.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebframe.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage_p.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/tests/qwebpage/tst_qwebpage.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebsecurityorigin.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.h"
Total errors found: 1 in 23 files
If any of these errors are false positives, please file a bug against check-webkit-style.
I tested with additional header, #include "SVGSMILElement.h", but unfortunately I got 7 failing tests. I will submit the diff here. Please add this include, and follow advices of style bot. :) If you run WebKitTools/Scripts/check-webkit-style locally before upload the patch, you can spare with resources of style bot. Created attachment 51248 [details]
Patch
Ossy, I'm pretty sure I've fixed any failures here. You might get layout differences but these should be pixel-widths only.
You might notice in the change to expected results for smart-delete-004 that the expected platform-specific result for qt has been changed from 'foo baz' to 'foo baz'. The latter is the expected result for mac. Created attachment 51254 [details] patch (for your patch) to make buildbot happy (In reply to comment #20) > Created an attachment (id=51248) [details] > Patch > > Ossy, I'm pretty sure I've fixed any failures here. You might get layout > differences but these should be pixel-widths only. I tested the last patch, the differences are only pixel differences, I attached the diff to make our buildbot happy. I'm not such a big fan of the refactoring: 1) before the webpage related methods were in qwebpage.cpp and that made sense, as they sometime turn into real methods. 2) now some of the methods are prefixed with webPage and others related to QWebPage are not, very inconsistent. 3) would it make more sense to add these methods to the *Private classes? such as QWebPagePrivate? or create a QWebPageDRT? I opt for the first one; we could even prefix all methods with DRT if needed, like QWebFramePrivate::drtClearName(qwebframe); (In reply to comment #23) > I'm not such a big fan of the refactoring: > > 1) before the webpage related methods were in qwebpage.cpp and that made sense, > as they sometime turn into real methods. You have to admit though that on the other hand speading all those drt methods throughout the API is a big mess. > 2) now some of the methods are prefixed with webPage and others related to > QWebPage are not, very inconsistent. That sounds fixable (for a follow-up patch). > 3) would it make more sense to add these methods to the *Private classes? such > as QWebPagePrivate? or create a QWebPageDRT? I opt for the first one; we could > even prefix all methods with DRT if needed, like > QWebFramePrivate::drtClearName(qwebframe); What's the advantage of grouping them in QWebPagePrivate if some of them operate on frames only and others are global functions? It seems just as unrelated as having a dedicated DRT class, that at least makes it clear that this is private DRT API, nothing else. (In reply to comment #24) > (In reply to comment #23) > > I'm not such a big fan of the refactoring: > > > > 1) before the webpage related methods were in qwebpage.cpp and that made sense, > > as they sometime turn into real methods. > > You have to admit though that on the other hand speading all those drt methods > throughout the API is a big mess. True to some degree, but they are related to the others methods where it is being implemented. Just like we implement QWebPagePrivate methods inside qwebpage.cpp; so the methods are grouped with related methods. > What's the advantage of grouping them in QWebPagePrivate if some of them > operate on frames only and others are global functions? The global ones we could just implement directly in the DumpRenderTree.cpp right? > It seems just as > unrelated as having a dedicated DRT class, that at least makes it clear that > this is private DRT API, nothing else. That is why I suggested creating QWebPageDRT and QWebFrameDRT, though that might be overkill. Anyway, the patch instantiates a QDRTSupport class, even though all methods basically act like static methods. If we do a refactoring we should discuss it, and do it part per part. I'm sure a lot of this code can actually be cleaned up. Anyway, the reason we are doing these classes are that DRT doesn't depend on WebCore; it is basically a workaround to the point that the DRT should only use our public API, right? I think the refactoring should be separate from this patch, don't you agree? (In reply to comment #26) > Anyway, the reason we are doing these classes are that DRT doesn't depend on > WebCore; it is basically a workaround to the point that the DRT should only use > our public API, right? > There's no reason why most of it couldn't be public undocumented API. If there is a reason for keeping it private then this refactoring or something similar seems to be required long term. So is there a good reason for keeping it private? (In reply to comment #25) >Anyway, the patch instantiates a QDRTSupport class, even though all methods >basically act like static methods. Yes, I should probably just make all the methods static and skip the instantiation. No good reason I didn't do that in the first place. Created attachment 51363 [details]
Updated Patch
Use static members from QDRTSupport rather than instantiating the class.
(In reply to comment #26) > I think the refactoring should be separate from this patch, don't you agree? That's true, I agree with that. Robert, could you separate it? Just leave out the smart insertion. Please send me a private email as a reminder, so that I can be a bit quicker in the review of the refactoring (sorry :-( Created attachment 52416 [details]
Refactor DRT Support
As per Kenneth and Simon's request - patch with DRT support refactoring only.
Created attachment 52417 [details]
Removed stray changelog entry
Oops, stray changelog entry.
Comment on attachment 52417 [details]
Removed stray changelog entry
As it is an internal class we should call it QtDRTSupport (or QtDrtSupport to following Qt naming conventions), as we do with most other internal Qt classes (not privates) in WebKit.
We still need to look at the method names. For instance it is not obvious what QtDRTSupport::run() does. Also, I would like you to group the methods logically together in the header. Also, you have methods in the header with signature (QWebFrame* qFrame, ...), please leave our the qFrame as the style guide dictates.
Created attachment 53050 [details]
Updated Patch
I've renamed it DumpRenderTreeSupportQt to match the filename. I've also rejigged one or two of the function names and reordered the functions in the header.
Comment on attachment 53050 [details] Updated Patch Clearing flags on attachment: 53050 Committed r57433: <http://trac.webkit.org/changeset/57433> All reviewed patches have been landed. Closing bug. Buildfix for --debug build landed: http://trac.webkit.org/changeset/57447 (In reply to comment #36) > Buildfix for --debug build landed: http://trac.webkit.org/changeset/57447 Sorry, I mean http://trac.webkit.org/changeset/57448 :) Re-opening to add layouttestcontroller commands. Created attachment 53084 [details]
LayoutTestController Commands Patch
(In reply to comment #33) > Created an attachment (id=53050) [details] > Updated Patch > > I've renamed it DumpRenderTreeSupportQt to match the filename. I've also > rejigged one or two of the function names and reordered the functions in the > header. patch 53050 that is now landed removed static function qt_wrt_setViewMode from qwebpage. It makes no sense to me since the only relation it has with DRT is the fact that there is a LayoutTest for viewModes. Now the hidden api used to change view modes is a static method of class DumpRenderTreeSupportQt. Was it actually the intention of the patch? (In reply to comment #40) > > Now the hidden api used to change view modes is a static method of class > DumpRenderTreeSupportQt. Was it actually the intention of the patch? It sounds like it was inappropriate to move it and that it should be moved back. (In reply to comment #41) > (In reply to comment #40) > > > > Now the hidden api used to change view modes is a static method of class > > DumpRenderTreeSupportQt. Was it actually the intention of the patch? > > It sounds like it was inappropriate to move it and that it should be moved > back. bug 37505 moves it back. http://trac.webkit.org/changeset/57793 might have broken Qt Linux Release (In reply to comment #43) > http://trac.webkit.org/changeset/57793 might have broken Qt Linux Release Fixed by http://trac.webkit.org/changeset/57794 , thx Robert. |
Add support for LayoutTestController commands: setSmartInsertDeleteEnabled setSelectTrailingWhitespaceEnabled execCommand isCommandEnabled Unskip tests: editing/deleting/smart-editing-disabled.html editing/execCommand/19089.html editing/execCommand/delete-image-in-anchor.html editing/execCommand/enabling-and-selection-2.html editing/selection/doubleclick-whitespace-crash.html editing/selection/doubleclick-whitespace-img-crash.html editing/selection/doubleclick-whitespace.html editing/selection/select-line.html Add Qt-specific results for tests: editing/deleting/5300379.html editing/deleting/smart-delete-003.html editing/deleting/smart-delete-004.html editing/selection/5195166-1.html editing/selection/5195166-2.html (These last are render tree tests and the only differences are minor pixel/width differences)