WebKitTestRunner needs layoutTestController.setEditingBehavior
<rdar://problem/8213852>
Seems like a good candidate to move to window.internal.
*** Bug 81738 has been marked as a duplicate of this bug. ***
Added editing/selection/move-by-word-visually-crash-test-5.html to the WebKit2 skip list in <http://trac.webkit.org/r111498>.
(In reply to comment #2) > Seems like a good candidate to move to window.internal. I'll give it a shot.
Created attachment 142581 [details] Patch
Comment on attachment 142581 [details] Patch Attachment 142581 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12718698
Comment on attachment 142581 [details] Patch Attachment 142581 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12722652 New failing tests: fast/dom/Document/CaretRangeFromPoint/caretRangeFromPoint-in-zoom-and-scroll.html fast/repaint/japanese-rl-selection-repaint.html fast/text/international/khmer-selection.html editing/selection/after-line-break.html
Created attachment 142619 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
CC'ing Alexey. similar my patch made a build problem yesterday. So, in my humble opinion, this patch needs to be reviewed by many reviewers as well.
Comment on attachment 142581 [details] Patch To avoid build break on gtk, win ports, you have to add add symbol to symbol filters. - Source/autotools/symbols.filter - Source/WebKit2/win/WebKit2.def
Comment on attachment 142581 [details] Patch Removing r? until I fix the problems already pointed out.
Comment on attachment 142581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142581&action=review Thanks for taking this! This is one of typical methods which should be on Internals object. I added a few comment. > Source/WebCore/testing/InternalSettings.cpp:343 > + fprintf(stderr, "setEditingBehavior(): Passed invalid editing behavior. Should be 'mac', 'win', or 'unix'.\n"); Instead of printing the error message, You can throw an exception by setting "ec" parameter. > Tools/ChangeLog:3 > + Move setEditingBehavior() from layoutTestController to window.internals The diff looks broken.
> similar my patch made a build problem yesterday. That patch removed a private Mac API, which is not happening here. As far as I can tell, it should be OK to remove setEditingBehavior from WebPreferencesPrivate.h.
Created attachment 142776 [details] Patch
(In reply to comment #14) > As far as I can tell, it should be OK to remove setEditingBehavior from WebPreferencesPrivate.h. Alexey, the patch currently doesn't remove it, but it could. Who should I ask about it?
Changed the approach a little bit. InternalSettings now store the previous value and restore it when resetting, instead of looking for default value. I changed the platform/wk2/Skipped based on Qt WK2 and Gtk WK2. I couldn't test in Mac WK2 yet.
> Alexey, the patch currently doesn't remove it, but it could. Who should I ask about it? I'm not sure if I understand the question. Yes, it seems fine to remove setEditingBehavior from WebPreferencesPrivate.h, but this is only what I said before.
Comment on attachment 142776 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142776&action=review Very nice patch! > Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:657 > +void DumpRenderTreeSupportQt::setEditingWindowsBehavior(QWebPage* page) setWindowsEditingBehavior ? useWindowsBehaviorAsEditingBehavior ? I think it could be a bit more clear > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:311 > + // FIXME: Remove this once blackberry calls resetInternalsObject(). bug number?
Created attachment 142809 [details] Patch
Comment on attachment 142809 [details] Patch Checking if Mac and Win will be happy with the changes.
Created attachment 142813 [details] Patch trying to make win bot apply the patch
Comment on attachment 142813 [details] Patch Looks good. I hope we could see the green win build.
Lucas, since you are the win bot admin you might know this: only win ews can't apply the patch, do you happen to know what's going on? The file in question is LayoutTests/fast/forms/selection-direction.html and uses "DOS Line Ending". I've tried both converting to Unix line ending or keeping the DOS line ending (the last two patches). Neither approach works. I am using git-svn.
...and webkit-patch upload, to upload these patches.
Created attachment 142993 [details] Patch
Committed r117771: <http://trac.webkit.org/changeset/117771>
Comment on attachment 142993 [details] Patch Clearing flags.
Comment on attachment 142993 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142993&action=review > LayoutTests/editing/deleting/delete-ligature-003.html:18 > - if(window.layoutTestController) > - layoutTestController.setEditingBehavior(platform); > + if (window.internals) > + window.internals.settings.setEditingBehavior(platform); No need for the "window." on the second line; just like the old code it can be just internals since we know it’s there. I noticed this patch adding “window.” prefixes in many places we don’t need to, which is a shame since “internals” would work fine alone.
> No need for the "window." on the second line; just like the old code it can be just internals since we know it’s there. I noticed this patch adding “window.” prefixes in many places we don’t need to, which is a shame since “internals” would work fine alone. I'll fix those extra "window." that were added by this patch. Sorry for the noise.
(In reply to comment #30) > I'll fix those extra "window." that were added by this patch. Sorry for the noise. Bug 87059.