Bug 143958

Summary: Context menu doesn't account for selection semantics
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, rniwa
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 144021    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews102 for mac-mavericks
none
Archive of layout-test-results from ews107 for mac-mavericks-wk2
none
Patch v2 (Test Fixes)
none
Archive of layout-test-results from ews103 for mac-mavericks
none
Archive of layout-test-results from ews104 for mac-mavericks-wk2
none
Patch (v3 Don't touch regular selection) thorton: review+

Description Brent Fulgham 2015-04-20 11:59:47 PDT
The right-click context menu only considers the word directly under the mouse. Instead, we should use the Lookup module (like we do with immediate actions) so that we get a contextually-relevant selection.
Comment 1 Brent Fulgham 2015-04-20 12:00:11 PDT
<rdar://problem/19735706>
Comment 2 Brent Fulgham 2015-04-21 11:26:22 PDT
Created attachment 251247 [details]
Patch
Comment 3 Build Bot 2015-04-21 12:02:12 PDT
Comment on attachment 251247 [details]
Patch

Attachment 251247 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4708163676798976

Number of test failures exceeded the failure limit.
Comment 4 Build Bot 2015-04-21 12:02:16 PDT
Created attachment 251249 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 5 Brent Fulgham 2015-04-21 12:11:38 PDT
Uh oh! Looks like I introduced some crashes. Will fix.
Comment 6 Build Bot 2015-04-21 12:14:44 PDT
Comment on attachment 251247 [details]
Patch

Attachment 251247 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4863398290391040

Number of test failures exceeded the failure limit.
Comment 7 Build Bot 2015-04-21 12:14:48 PDT
Created attachment 251253 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 8 Brent Fulgham 2015-04-21 15:02:57 PDT
Committed r183077: <http://trac.webkit.org/changeset/183077>
Comment 9 WebKit Commit Bot 2015-04-21 16:21:21 PDT
Re-opened since this is blocked by bug 144021
Comment 10 Brent Fulgham 2015-04-22 11:34:39 PDT
Created attachment 251345 [details]
Patch v2 (Test Fixes)
Comment 11 Brent Fulgham 2015-04-22 11:36:46 PDT
Comment on attachment 251345 [details]
Patch v2 (Test Fixes)

Revised patch as follows:
1. Dictionary lookup was not honoring standard WebKit editing rules for handling hit tests at end-of-line, end-of-paragraph, etc.
2. The new dictionary lookup was not retaining trailing spaces when asked to by the system.
3. Rebaselined one test for the new dictionary-lookup behavior.
Comment 12 Brent Fulgham 2015-04-22 11:50:55 PDT
Looks like I'll also need to rebaseline some tests based on the Mavericks bot output.
Comment 13 Build Bot 2015-04-22 11:56:13 PDT
Comment on attachment 251345 [details]
Patch v2 (Test Fixes)

Attachment 251345 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6495054554202112

New failing tests:
editing/selection/select-from-textfield-outwards.html
editing/selection/doubleclick-beside-cr-span.html
Comment 14 Build Bot 2015-04-22 11:56:15 PDT
Created attachment 251348 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 15 Build Bot 2015-04-22 12:14:41 PDT
Comment on attachment 251345 [details]
Patch v2 (Test Fixes)

Attachment 251345 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6563398422626304

New failing tests:
editing/selection/select-from-textfield-outwards.html
Comment 16 Build Bot 2015-04-22 12:14:44 PDT
Created attachment 251355 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 17 Brent Fulgham 2015-04-22 14:26:50 PDT
Created attachment 251367 [details]
Patch (v3 Don't touch regular selection)
Comment 18 Brent Fulgham 2015-04-22 14:48:05 PDT
Comment on attachment 251367 [details]
Patch (v3 Don't touch regular selection)

Revised the patch to only affect context menu operations. This corrected the five failing tests.
Comment 19 Tim Horton 2015-04-22 14:53:22 PDT
Comment on attachment 251367 [details]
Patch (v3 Don't touch regular selection)

You could probably write a test.
Comment 20 Brent Fulgham 2015-04-22 15:11:35 PDT
Committed r183138: <http://trac.webkit.org/changeset/183138>
Comment 21 Brent Fulgham 2015-04-22 15:35:42 PDT
(In reply to comment #19)
> Comment on attachment 251367 [details]
> Patch (v3 Don't touch regular selection)
> 
> You could probably write a test.

Test added during landing! :-)