Bug 176096

Summary: Unpredictable selection when dragging out of float elements children of in-flow block-level box
Product: WebKit Reporter: Javier Fernandez <jfernandez>
Component: HTML EditingAssignee: Javier Fernandez <jfernandez>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, dbates, hyatt, jbedard, jfernandez, rniwa, simon.fraser, webkit-bug-importer, wenson_hsieh, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.chromium.org/p/chromium/issues/detail?id=758526
Bug Depends on:    
Bug Blocks: 101771    
Attachments:
Description Flags
Test case to reproduce the bug
none
Patch
none
Archive of layout-test-results from ews101 for mac-elcapitan
none
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews116 for mac-elcapitan
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Patch
none
Patch
none
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews101 for mac-elcapitan
none
Archive of layout-test-results from ews116 for mac-elcapitan
none
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews123 for ios-simulator-wk2
none
Patch none

Description Javier Fernandez 2017-08-30 03:59:43 PDT
Created attachment 319356 [details]
Test case to reproduce the bug

This bug is the first one of several issues we have with selection and out-of-flow elements. 

Attached a reduced case to reproduce the issue. Selecting from any point inside the float element and dragging outside its visible area will cause the selection to change it's visible boundaries, in a very weird and unpredictable way. 

This particular issue address the case of float elements children of an in-flow block-level box. In this scenario, we use the LayoutBlock hit testing logic to determine the position of the selection point. Since we exclude float elements from the valid candidates, we end up setting the parent node. This logic implies that original start/end selection points will be transposed.
Comment 1 Javier Fernandez 2017-09-05 08:31:54 PDT
Created attachment 319897 [details]
Patch
Comment 2 Build Bot 2017-09-05 09:39:04 PDT
Comment on attachment 319897 [details]
Patch

Attachment 319897 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4454792

New failing tests:
fast/multicol/hit-test-above-or-below.html
fast/writing-mode/positionForPoint.html
fast/events/drag-and-drop-link-into-focused-contenteditable.html
editing/selection/click-in-margins-inside-editable-div.html
Comment 3 Build Bot 2017-09-05 09:39:05 PDT
Created attachment 319906 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 4 Build Bot 2017-09-05 09:43:51 PDT
Comment on attachment 319897 [details]
Patch

Attachment 319897 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4454798

New failing tests:
fast/multicol/hit-test-above-or-below.html
fast/writing-mode/positionForPoint.html
editing/selection/click-in-margins-inside-editable-div.html
Comment 5 Build Bot 2017-09-05 09:43:53 PDT
Created attachment 319909 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 6 Build Bot 2017-09-05 09:58:01 PDT
Comment on attachment 319897 [details]
Patch

Attachment 319897 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4454826

New failing tests:
fast/multicol/hit-test-above-or-below.html
fast/writing-mode/positionForPoint.html
fast/events/drag-and-drop-link-into-focused-contenteditable.html
editing/selection/click-in-margins-inside-editable-div.html
Comment 7 Build Bot 2017-09-05 09:58:03 PDT
Created attachment 319910 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 8 Build Bot 2017-09-05 10:07:54 PDT
Comment on attachment 319897 [details]
Patch

Attachment 319897 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4454825

New failing tests:
editing/selection/select-out-of-floated-non-editable-05.html
editing/selection/select-out-of-floated-non-editable-10.html
editing/selection/select-out-of-floated-non-editable-01.html
fast/writing-mode/positionForPoint.html
editing/selection/select-out-of-floated-non-editable-12.html
editing/selection/select-out-of-floated-non-editable-04.html
editing/selection/select-out-of-floated-non-editable-09.html
fast/multicol/hit-test-above-or-below.html
editing/selection/select-out-of-floated-non-editable-07.html
editing/selection/select-out-of-floated-non-editable-06.html
editing/selection/select-out-of-floated-non-editable-02.html
editing/selection/select-out-of-floated-non-editable-08.html
editing/selection/select-out-of-floated-non-editable-11.html
editing/selection/select-out-of-floated-non-editable-03.html
Comment 9 Build Bot 2017-09-05 10:07:56 PDT
Created attachment 319912 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 10 Javier Fernandez 2017-09-06 02:42:08 PDT
Created attachment 319994 [details]
Patch
Comment 11 Javier Fernandez 2017-09-06 02:55:39 PDT
Created attachment 319996 [details]
Patch
Comment 12 Build Bot 2017-09-06 03:51:55 PDT
Comment on attachment 319996 [details]
Patch

Attachment 319996 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4461750

New failing tests:
fast/multicol/hit-test-above-or-below.html
editing/selection/select-out-of-floated-non-editable-07.html
editing/selection/select-out-of-floated-non-editable-12.html
editing/selection/click-in-margins-inside-editable-div.html
editing/selection/select-out-of-floated-non-editable-09.html
Comment 13 Build Bot 2017-09-06 03:51:56 PDT
Created attachment 320002 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 14 Build Bot 2017-09-06 04:03:52 PDT
Comment on attachment 319996 [details]
Patch

Attachment 319996 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4461893

New failing tests:
editing/selection/select-out-of-floated-non-editable-12.html
fast/events/drag-and-drop-link-into-focused-contenteditable.html
editing/selection/select-out-of-floated-non-editable-09.html
fast/multicol/hit-test-above-or-below.html
editing/selection/select-out-of-floated-non-editable-07.html
editing/selection/click-in-margins-inside-editable-div.html
Comment 15 Build Bot 2017-09-06 04:03:54 PDT
Created attachment 320003 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 16 Build Bot 2017-09-06 04:18:23 PDT
Comment on attachment 319996 [details]
Patch

Attachment 319996 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4461898

New failing tests:
editing/selection/select-out-of-floated-non-editable-12.html
fast/events/drag-and-drop-link-into-focused-contenteditable.html
editing/selection/select-out-of-floated-non-editable-09.html
fast/multicol/hit-test-above-or-below.html
editing/selection/select-out-of-floated-non-editable-07.html
editing/selection/click-in-margins-inside-editable-div.html
Comment 17 Build Bot 2017-09-06 04:18:24 PDT
Created attachment 320004 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 18 Build Bot 2017-09-06 04:33:22 PDT
Comment on attachment 319996 [details]
Patch

Attachment 319996 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4462000

New failing tests:
editing/selection/select-out-of-floated-non-editable-05.html
editing/selection/select-out-of-floated-non-editable-10.html
editing/selection/select-out-of-floated-non-editable-01.html
editing/selection/select-out-of-floated-non-editable-12.html
editing/selection/select-out-of-floated-non-editable-04.html
editing/selection/select-out-of-floated-non-editable-09.html
fast/multicol/hit-test-above-or-below.html
editing/selection/select-out-of-floated-non-editable-07.html
editing/selection/select-out-of-floated-non-editable-06.html
editing/selection/select-out-of-floated-non-editable-02.html
editing/selection/select-out-of-floated-non-editable-08.html
editing/selection/select-out-of-floated-non-editable-11.html
editing/selection/select-out-of-floated-non-editable-03.html
Comment 19 Build Bot 2017-09-06 04:33:23 PDT
Created attachment 320007 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 20 Javier Fernandez 2017-09-06 15:54:26 PDT
Created attachment 320070 [details]
Patch
Comment 21 Build Bot 2017-09-06 17:34:13 PDT
Comment on attachment 320070 [details]
Patch

Attachment 320070 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4468830

New failing tests:
editing/selection/select-out-of-floated-non-editable-05.html
editing/selection/select-out-of-floated-non-editable-10.html
editing/selection/select-out-of-floated-non-editable-01.html
editing/selection/select-out-of-floated-non-editable-12.html
editing/selection/select-out-of-floated-non-editable-04.html
editing/selection/select-out-of-floated-non-editable-09.html
editing/selection/select-out-of-floated-non-editable-07.html
editing/selection/select-out-of-floated-non-editable-06.html
editing/selection/select-out-of-floated-non-editable-02.html
editing/selection/select-out-of-floated-non-editable-08.html
editing/selection/select-out-of-floated-non-editable-11.html
editing/selection/select-out-of-floated-non-editable-03.html
Comment 22 Build Bot 2017-09-06 17:34:14 PDT
Created attachment 320082 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 23 Dave Hyatt 2017-09-13 13:45:23 PDT
Comment on attachment 320070 [details]
Patch

r=me
Comment 24 Ryosuke Niwa 2017-09-13 14:02:08 PDT
Comment on attachment 320070 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320070&action=review

> Source/WebCore/rendering/RenderBlock.cpp:2714
> +                || (blocksAreFlipped && pointInLogicalContents.y() == childLogicalBottom))) {
>                  return positionForPointRespectingEditingBoundaries(*this, *childBox, pointInContents);
> +            }

Nit: This doesn't match our style guideline. PLEASE DON'T ADD curly braces.
Comment 25 Javier Fernandez 2017-09-18 00:43:06 PDT
Created attachment 321081 [details]
Patch
Comment 26 Build Bot 2017-09-18 02:22:28 PDT
Comment on attachment 321081 [details]
Patch

Attachment 321081 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4581447

New failing tests:
editing/selection/select-out-of-floated-non-editable-05.html
editing/selection/select-out-of-floated-non-editable-10.html
editing/selection/select-out-of-floated-non-editable-01.html
editing/selection/select-out-of-floated-non-editable-12.html
editing/selection/select-out-of-floated-non-editable-04.html
editing/selection/select-out-of-floated-non-editable-09.html
editing/selection/select-out-of-floated-non-editable-07.html
editing/selection/select-out-of-floated-non-editable-06.html
editing/selection/select-out-of-floated-non-editable-02.html
editing/selection/select-out-of-floated-non-editable-08.html
editing/selection/select-out-of-floated-non-editable-11.html
editing/selection/select-out-of-floated-non-editable-03.html
Comment 27 Build Bot 2017-09-18 02:22:29 PDT
Created attachment 321085 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 28 Javier Fernandez 2017-09-20 01:52:38 PDT
Could anybody give any hint about the reason of this io-simulator specific failures ? It looks like the eventSender based selection is not working at all in the simulator. Is that the case ?
Comment 29 Wenson Hsieh 2017-09-20 08:38:01 PDT
(In reply to Javier Fernandez from comment #28)
> Could anybody give any hint about the reason of this io-simulator specific
> failures ? It looks like the eventSender based selection is not working at
> all in the simulator. Is that the case ?

Unfortunately, the EventSender.mouseDown() and friends are empty stubs on iOS WebKit2 (see Tools/WebKitTestRunner/InjectedBundle/ios/EventSenderProxyIOS.mm); there are a few existing selection-related LayoutTests that use UIScriptController to synthesize text selection gestures instead, but those don't run in OpenSource.

I think you'll need to mark these as [ Fail ] or [ Skip ] in LayoutTests/platform/ios-wk2/TestExpectations.
Comment 30 Simon Fraser (smfr) 2017-09-20 08:52:16 PDT
Right, EventSender does not simulate mouse events on iOS. You should probably skip the tests on iOS.
Comment 31 Wenson Hsieh 2017-09-20 08:53:41 PDT
(In reply to Wenson Hsieh from comment #29)
> (In reply to Javier Fernandez from comment #28)
> > Could anybody give any hint about the reason of this io-simulator specific
> > failures ? It looks like the eventSender based selection is not working at
> > all in the simulator. Is that the case ?
> 
> Unfortunately, the EventSender.mouseDown() and friends are empty stubs on
> iOS WebKit2 (see
> Tools/WebKitTestRunner/InjectedBundle/ios/EventSenderProxyIOS.mm); there are
> a few existing selection-related LayoutTests that use UIScriptController to
> synthesize text selection gestures instead, but those don't run in
> OpenSource.

For some examples, see: editing/selection/character-granularity-rect.html, long-press-then-drag-down-to-change-selected-text.html.

UIScriptController.longPressAtPoint() is supported in OpenSource and can be used to select text, but any tests that additionally depend on tap gestures (e.g. to begin editing text fields) are skipped.
Comment 32 Javier Fernandez 2017-09-21 02:34:45 PDT
Created attachment 321417 [details]
Patch

Skip tests on the iOS simulator
Comment 33 WebKit Commit Bot 2017-09-21 06:43:35 PDT
Comment on attachment 321417 [details]
Patch

Clearing flags on attachment: 321417

Committed r222317: <http://trac.webkit.org/changeset/222317>
Comment 34 WebKit Commit Bot 2017-09-21 06:43:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 35 Radar WebKit Bug Importer 2017-09-27 12:53:51 PDT
<rdar://problem/34694233>
Comment 36 Jonathan Bedard 2018-01-19 09:15:28 PST
The iOS Simulator expectations committed in <https://trac.webkit.org/changeset/222317/webkit> should have been placed in the iOS expectations directory. Move them in <https://trac.webkit.org/changeset/227202/webkit>.