Bug 94449

Summary: [TouchAdjustment] Adjust to word or selection
Product: WebKit Reporter: Allan Sandfeld Jensen <allan.jensen>
Component: UI EventsAssignee: Allan Sandfeld Jensen <allan.jensen>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, gmak, kbr, mifenton, tdanderson, tonikitoo, webkit.review.bot
Priority: P2    
Version: 420+   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from gce-cr-linux-03
none
Patch
none
Patch none

Allan Sandfeld Jensen
Reported 2012-08-20 00:45:13 PDT
The context-menu gesture is currently not adjusting as good as it could, to the nearest word, or the active selection.
Attachments
Patch (8.74 KB, patch)
2012-08-20 00:49 PDT, Allan Sandfeld Jensen
no flags
Archive of layout-test-results from gce-cr-linux-03 (554.60 KB, application/zip)
2012-08-20 02:31 PDT, WebKit Review Bot
no flags
Patch (16.59 KB, patch)
2012-08-20 02:50 PDT, Allan Sandfeld Jensen
no flags
Patch (16.58 KB, patch)
2012-08-22 02:25 PDT, Allan Sandfeld Jensen
no flags
Allan Sandfeld Jensen
Comment 1 2012-08-20 00:49:58 PDT
Allan Sandfeld Jensen
Comment 2 2012-08-20 00:51:18 PDT
Comment on attachment 159350 [details] Patch Needs a test-case before landing.
WebKit Review Bot
Comment 3 2012-08-20 02:31:44 PDT
Comment on attachment 159350 [details] Patch Attachment 159350 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13529890 New failing tests: touchadjustment/context-menu-select-text.html
WebKit Review Bot
Comment 4 2012-08-20 02:31:49 PDT
Created attachment 159378 [details] Archive of layout-test-results from gce-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Allan Sandfeld Jensen
Comment 5 2012-08-20 02:50:23 PDT
Created attachment 159381 [details] Patch Added test, and fixed the word-iteration
WebKit Review Bot
Comment 6 2012-08-20 07:50:40 PDT
Comment on attachment 159381 [details] Patch Clearing flags on attachment: 159381 Committed r126026: <http://trac.webkit.org/changeset/126026>
WebKit Review Bot
Comment 7 2012-08-20 07:50:44 PDT
All reviewed patches have been landed. Closing bug.
Kenneth Russell
Comment 8 2012-08-20 13:42:27 PDT
This patch caused assertion failures inside WebCore::TouchAdjustment::appendContextSubtargetsForNode(WebCore::Node*, WebCore::TouchAdjustment::SubtargetGeometryList&): crash log for DumpRenderTree (pid 26367): STDOUT: <empty> STDERR: ASSERTION FAILED: textRenderer->selectionState() != RenderObject::SelectionNone STDERR: third_party/WebKit/Source/WebCore/page/TouchAdjustment.cpp(168) : void WebCore::TouchAdjustment::appendContextSubtargetsForNode(WebCore::Node*, WebCore::TouchAdjustment::SubtargetGeometryList&) STDERR: 1 0x7f634be7a329 STDERR: 2 0x7f634be7a9a9 STDERR: 3 0x7f634be7ba10 STDERR: 4 0x7f634be0cb15 STDERR: 5 0x7f634aec1bbb STDERR: 6 0x7f634aed42d9 STDERR: 7 0x7f63483d46f4 STDERR: 8 0x7f63483cf50d STDERR: 9 0x7f63483cf4de STDERR: 10 0x28b7ece0618e STDERR: [26367:26367:12957102334383:ERROR:process_util_posix.cc(143)] Received signal 11 STDERR: base::debug::StackTrace::StackTrace() [0x7f6347f58ea6] STDERR: base::(anonymous namespace)::StackDumpSignalHandler() [0x7f6347fbfffd] STDERR: 0x7f6341c4aaf0 STDERR: WebCore::TouchAdjustment::appendContextSubtargetsForNode() [0x7f634be7a333] STDERR: WebCore::TouchAdjustment::compileSubtargetList() [0x7f634be7a9a9] STDERR: WebCore::findBestContextMenuCandidate() [0x7f634be7ba10] STDERR: WebCore::EventHandler::bestContextMenuNodeForTouchPoint() [0x7f634be0cb15] STDERR: WebCore::Internals::touchNodeAdjustedToBestContextMenuNode() [0x7f634aec1bbb] STDERR: WebCore::InternalsV8Internal::touchNodeAdjustedToBestContextMenuNodeCallback() [0x7f634aed42d9] STDERR: v8::internal::HandleApiCallHelper<>() [0x7f63483d46f4] STDERR: v8::internal::Builtin_Impl_HandleApiCall() [0x7f63483cf50d] STDERR: v8::internal::Builtin_HandleApiCall() [0x7f63483cf4de] STDERR: 0x28b7ece0618e See: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=touchadjustment%2Fcontext-menu.html I'm inclined to roll out this patch because this may actually be a real problem with the code. Will try to contact the author or reviewer on IRC first.
Kenneth Russell
Comment 9 2012-08-20 14:13:28 PDT
Pinged tonikitoo on IRC about this but wasn't able to reach carewolf. I'm rolling out this patch because of potentially real problems in the code -- suppressing the assertion failure in TestExpectations seems like a bad idea.
Kenneth Russell
Comment 10 2012-08-20 14:17:21 PDT
Reverted r126026 for reason: Caused assertion failure in layout test touchadjustment/context-menu.html Committed r126069: <http://trac.webkit.org/changeset/126069>
Allan Sandfeld Jensen
Comment 11 2012-08-20 14:25:48 PDT
The assertion is wrong, and the case asserted was actually handled in the final patch. I will remove the wrong assertion in a new version of the patch.
Allan Sandfeld Jensen
Comment 12 2012-08-22 02:25:57 PDT
WebKit Review Bot
Comment 13 2012-08-22 03:23:33 PDT
Comment on attachment 159888 [details] Patch Clearing flags on attachment: 159888 Committed r126284: <http://trac.webkit.org/changeset/126284>
WebKit Review Bot
Comment 14 2012-08-22 03:23:37 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.