Changeset 234661 in webkit


Ignore:
Timestamp:
Aug 7, 2018, 11:32:42 AM (7 years ago)
Author:
Wenson Hsieh
Message:

REGRESSION (r233778): Text selection sometimes cannot be extended in iframes
​https://bugs.webkit.org/show_bug.cgi?id=188374
<rdar://problem/42928657>

Reviewed by Simon Fraser.

Source/WebKit:

rangeForPoint contains logic for converting a selection handle location in root view coordinates to an updated
selection. In doing so, we first convert the selection handle location to content coordinates; however, the call
site to EventHandler::hitTestResultAtPoint still hit-tests using the location in root view coordinates rather
than content coordinates, which means that when the focused frame is a subframe, hit-testing will fail to find
nodes within the subframe under the selection handle. This manifests in behaviors such as snapping to a single
character when selecting text in subframes.

To fix this, we just need to pass in the point in the frame's content coordinates when hit-testing.

Tests: editing/selection/ios/selection-handles-in-iframe.html

editing/selection/ios/selection-handles-in-readonly-input.html

  • WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::rangeForPointInRootViewCoordinates):

Make a couple of other minor adjustments:

  1. Take a Frame& instead of a Frame*, since Frame& is assumed to be non-null here.
  2. Rename rangeForPoint to rangeForPointInRootViewCoordinates, as well as the point argument to

pointInRootViewCoordinates.

(WebKit::WebPage::updateSelectionWithTouches):
(WebKit::rangeForPoint): Deleted.

LayoutTests:

Add 2 new layout tests to cover the original bug that r233778 fixed, as well as the regression in this bug.

  • editing/selection/ios/selection-handles-in-iframe-expected.txt: Added.
  • editing/selection/ios/selection-handles-in-iframe.html: Added.

Add a test to verify that the user can select text in an iframe by dragging selection handles.

  • editing/selection/ios/selection-handles-in-readonly-input-expected.txt: Added.
  • editing/selection/ios/selection-handles-in-readonly-input.html: Added.

Add a test to verify that dragging a selection handle outside of a readonly input does not cause the selection
to jump outside of the input and clear out the selection in the input.

Location:
trunk
Files:
4 added
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r234658 r234661  
     12018-08-07  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r233778): Text selection sometimes cannot be extended in iframes
     4        https://bugs.webkit.org/show_bug.cgi?id=188374
     5        <rdar://problem/42928657>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Add 2 new layout tests to cover the original bug that r233778 fixed, as well as the regression in this bug.
     10
     11        * editing/selection/ios/selection-handles-in-iframe-expected.txt: Added.
     12        * editing/selection/ios/selection-handles-in-iframe.html: Added.
     13
     14        Add a test to verify that the user can select text in an iframe by dragging selection handles.
     15
     16        * editing/selection/ios/selection-handles-in-readonly-input-expected.txt: Added.
     17        * editing/selection/ios/selection-handles-in-readonly-input.html: Added.
     18
     19        Add a test to verify that dragging a selection handle outside of a readonly input does not cause the selection
     20        to jump outside of the input and clear out the selection in the input.
     21
    1222018-08-07  Alex Christensen  <achristensen@webkit.org>
    223

  • trunk/Source/WebKit/ChangeLog

    r234658 r234661  
     12018-08-07  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r233778): Text selection sometimes cannot be extended in iframes
     4        https://bugs.webkit.org/show_bug.cgi?id=188374
     5        <rdar://problem/42928657>
     6
     7        Reviewed by Simon Fraser.
     8
     9        rangeForPoint contains logic for converting a selection handle location in root view coordinates to an updated
     10        selection. In doing so, we first convert the selection handle location to content coordinates; however, the call
     11        site to EventHandler::hitTestResultAtPoint still hit-tests using the location in root view coordinates rather
     12        than content coordinates, which means that when the focused frame is a subframe, hit-testing will fail to find
     13        nodes within the subframe under the selection handle. This manifests in behaviors such as snapping to a single
     14        character when selecting text in subframes.
     15
     16        To fix this, we just need to pass in the point in the frame's content coordinates when hit-testing.
     17
     18        Tests:  editing/selection/ios/selection-handles-in-iframe.html
     19                editing/selection/ios/selection-handles-in-readonly-input.html
     20
     21        * WebProcess/WebPage/ios/WebPageIOS.mm:
     22        (WebKit::rangeForPointInRootViewCoordinates):
     23
     24        Make a couple of other minor adjustments:
     25        1.  Take a Frame& instead of a Frame*, since Frame& is assumed to be non-null here.
     26        2.  Rename rangeForPoint to rangeForPointInRootViewCoordinates, as well as the point argument to
     27            pointInRootViewCoordinates.
     28
     29        (WebKit::WebPage::updateSelectionWithTouches):
     30        (WebKit::rangeForPoint): Deleted.
     31
    1322018-08-07  Alex Christensen  <achristensen@webkit.org>
    233

  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r234369 r234661  
    12161216}
    12171217
    1218 static RefPtr<Range> rangeForPoint(Frame* frame, const IntPoint& point, bool baseIsStart)
    1219 {
    1220     IntPoint pointInDocument = frame->view()->rootViewToContents(point);
    1221     Position result = frame->visiblePositionForPoint(pointInDocument).deepEquivalent();
     1218static RefPtr<Range> rangeForPointInRootViewCoordinates(Frame& frame, const IntPoint& pointInRootViewCoordinates, bool baseIsStart)
     1219{
     1220    IntPoint pointInDocument = frame.view()->rootViewToContents(pointInRootViewCoordinates);
     1221    Position result;
    12221222    RefPtr<Range> range;
    12231223   
    1224     HitTestResult hitTest = frame->eventHandler().hitTestResultAtPoint(point, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent);
     1224    HitTestResult hitTest = frame.eventHandler().hitTestResultAtPoint(pointInDocument, HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent);
    12251225    if (hitTest.targetNode())
    1226         result = frame->eventHandler().selectionExtentRespectingEditingBoundary(frame->selection().selection(), hitTest.localPoint(), hitTest.targetNode()).deepEquivalent();
    1227    
    1228     VisibleSelection existingSelection = frame->selection().selection();
     1226        result = frame.eventHandler().selectionExtentRespectingEditingBoundary(frame.selection().selection(), hitTest.localPoint(), hitTest.targetNode()).deepEquivalent();
     1227    else
     1228        result = frame.visiblePositionForPoint(pointInDocument).deepEquivalent();
     1229   
     1230    VisibleSelection existingSelection = frame.selection().selection();
    12291231    Position selectionStart = existingSelection.visibleStart().deepEquivalent();
    12301232    Position selectionEnd = existingSelection.visibleEnd().deepEquivalent();
    … …  
    12361238            result = VisibleSelection::adjustPositionForEnd(result, selectionStart.containerNode());
    12371239        if (result.isNotNull())
    1238             range = Range::create(*frame->document(), selectionStart, result);
     1240            range = Range::create(*frame.document(), selectionStart, result);
    12391241    } else {
    12401242        if (comparePositions(selectionEnd, result) <= 0)
    … …  
    12431245            result = VisibleSelection::adjustPositionForStart(result, selectionEnd.containerNode());
    12441246        if (result.isNotNull())
    1245             range = Range::create(*frame->document(), result, selectionEnd);
     1247            range = Range::create(*frame.document(), result, selectionEnd);
    12461248    }
    12471249   
    … …  
    13301332                range = Range::create(*frame.document(), result, result);
    13311333        } else
    1332             range = rangeForPoint(&frame, point, baseIsStart);
     1334            range = rangeForPointInRootViewCoordinates(frame, point, baseIsStart);
    13331335        break;
    13341336
    … …  
    13421344
    13431345    case SelectionTouch::Moved:
    1344         range = rangeForPoint(&frame, point, baseIsStart);
     1346        range = rangeForPointInRootViewCoordinates(frame, point, baseIsStart);
    13451347        break;
    13461348    }
Note: See TracChangeset for help on using the changeset viewer.