Changeset 242968 in webkit


Ignore:
Timestamp:
Mar 14, 2019 3:07:07 PM (5 years ago)
Author:
Wenson Hsieh
Message:

REGRESSION (r242801): [iOS] preventDefault() on touchstart in a subframe does not prevent focusing the subframe
https://bugs.webkit.org/show_bug.cgi?id=195749
<rdar://problem/48892367>

Reviewed by Tim Horton.

Source/WebKit:

r242801 added logic to fetch interaction information at the touch location upon touch start. However this,
combined with an existing behavior where the process of computing InteractionInformationAtPosition in WebPage
moves focus into the frame of the hit-tested node below the touch location, means that we'll always trigger a
blur event on the window and move focus into the subframe when performing a touch inside a subframe, even if the
page prevents default on touchstart.

To fix this, add a "readonly" flag to InteractionInformationRequest, and only change focus when requesting
position information in the case where the request is not readonly. For now, this readonly flag is false by
default; in a future patch, we should identify the (hopefully few) places that rely on position information
requests to move focus, explicitly turn this bit off in those places, and otherwise send readonly position
information requests by default.

  • Shared/ios/InteractionInformationRequest.cpp:

(WebKit::InteractionInformationRequest::encode const):
(WebKit::InteractionInformationRequest::decode):
(WebKit::InteractionInformationRequest::isValidForRequest):
(WebKit::InteractionInformationRequest::isApproximatelyValidForRequest):

Ensure that a readonly request is not valid for a non-readonly request.

  • Shared/ios/InteractionInformationRequest.h:
  • UIProcess/API/Cocoa/WKWebView.mm:

(-[WKWebView _requestActivatedElementAtPosition:completionBlock:]):

Send a readonly position information request in the case where a WebKit SPI client is querying for element
information at the given location.

  • UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView _webTouchEventsRecognized:]):

Send a readonly position information request on touchstart.

  • WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::positionInformation):

LayoutTests:

Add a test to verify that tapping a subframe doesn't move focus into it subframe if the page prevents default
on touchstart.

  • fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt: Added.
  • fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html: Added.
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r242956 r242968  
     12019-03-14  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r242801): [iOS] preventDefault() on touchstart in a subframe does not prevent focusing the subframe
     4        https://bugs.webkit.org/show_bug.cgi?id=195749
     5        <rdar://problem/48892367>
     6
     7        Reviewed by Tim Horton.
     8
     9        Add a test to verify that tapping a subframe doesn't move focus into it subframe if the page prevents default
     10        on touchstart.
     11
     12        * fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart-expected.txt: Added.
     13        * fast/events/touch/ios/no-focus-change-when-preventing-default-on-touchstart.html: Added.
     14
    1152019-03-14  Shawn Roberts  <sroberts@apple.com>
    216
  • trunk/Source/WebKit/ChangeLog

    r242960 r242968  
     12019-03-14  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r242801): [iOS] preventDefault() on touchstart in a subframe does not prevent focusing the subframe
     4        https://bugs.webkit.org/show_bug.cgi?id=195749
     5        <rdar://problem/48892367>
     6
     7        Reviewed by Tim Horton.
     8
     9        r242801 added logic to fetch interaction information at the touch location upon touch start. However this,
     10        combined with an existing behavior where the process of computing InteractionInformationAtPosition in WebPage
     11        moves focus into the frame of the hit-tested node below the touch location, means that we'll always trigger a
     12        blur event on the window and move focus into the subframe when performing a touch inside a subframe, even if the
     13        page prevents default on touchstart.
     14
     15        To fix this, add a "readonly" flag to InteractionInformationRequest, and only change focus when requesting
     16        position information in the case where the request is not readonly. For now, this readonly flag is false by
     17        default; in a future patch, we should identify the (hopefully few) places that rely on position information
     18        requests to move focus, explicitly turn this bit off in those places, and otherwise send readonly position
     19        information requests by default.
     20
     21        * Shared/ios/InteractionInformationRequest.cpp:
     22        (WebKit::InteractionInformationRequest::encode const):
     23        (WebKit::InteractionInformationRequest::decode):
     24        (WebKit::InteractionInformationRequest::isValidForRequest):
     25        (WebKit::InteractionInformationRequest::isApproximatelyValidForRequest):
     26
     27        Ensure that a readonly request is not valid for a non-readonly request.
     28
     29        * Shared/ios/InteractionInformationRequest.h:
     30        * UIProcess/API/Cocoa/WKWebView.mm:
     31        (-[WKWebView _requestActivatedElementAtPosition:completionBlock:]):
     32
     33        Send a readonly position information request in the case where a WebKit SPI client is querying for element
     34        information at the given location.
     35
     36        * UIProcess/ios/WKContentViewInteraction.mm:
     37        (-[WKContentView _webTouchEventsRecognized:]):
     38
     39        Send a readonly position information request on touchstart.
     40
     41        * WebProcess/WebPage/ios/WebPageIOS.mm:
     42        (WebKit::WebPage::positionInformation):
     43
    1442019-03-14  Chris Dumez  <cdumez@apple.com>
    245
  • trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp

    r237266 r242968  
    3939    encoder << includeSnapshot;
    4040    encoder << includeLinkIndicator;
     41    encoder << readonly;
    4142}
    4243
     
    5051
    5152    if (!decoder.decode(result.includeLinkIndicator))
     53        return false;
     54
     55    if (!decoder.decode(result.readonly))
    5256        return false;
    5357
     
    6670        return false;
    6771
     72    if (!other.readonly && readonly)
     73        return false;
     74
    6875    return true;
    6976}
     
    7683    if (other.includeLinkIndicator && !includeLinkIndicator)
    7784        return false;
     85
     86    if (!other.readonly && readonly)
     87        return false;
    7888   
    7989    return (other.point - point).diagonalLengthSquared() <= 4;
  • trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.h

    r237266 r242968  
    4343    bool includeLinkIndicator { false };
    4444
     45    // FIXME: This readonly flag ought to be true by default, but there are a number of interactions (e.g. selection) that currently
     46    // rely on the fact that interaction information requests additionally change the focused frame. We should explicitly turn the
     47    // readonly bit off in these scenarios, and make sure that all other position information requests don't move focus around.
     48    bool readonly { false };
     49
    4550    InteractionInformationRequest() { }
    4651    explicit InteractionInformationRequest(WebCore::IntPoint point)
  • trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

    r242762 r242968  
    65786578    auto infoRequest = WebKit::InteractionInformationRequest(WebCore::roundedIntPoint(position));
    65796579    infoRequest.includeSnapshot = true;
     6580    infoRequest.readonly = true;
    65806581
    65816582    [_contentView doAfterPositionInformationUpdate:[capturedBlock = makeBlockPtr(block)] (WebKit::InteractionInformationAtPosition information) {
  • trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

    r242833 r242968  
    12221222        _layerTreeTransactionIdAtLastTouchStart = downcast<WebKit::RemoteLayerTreeDrawingAreaProxy>(*_page->drawingArea()).lastCommittedLayerTreeTransactionID();
    12231223
     1224        WebKit::InteractionInformationRequest positionInformationRequest { WebCore::IntPoint(_lastInteractionLocation) };
     1225        positionInformationRequest.readonly = true;
    12241226        [self doAfterPositionInformationUpdate:[assistant = WeakObjCPtr<WKActionSheetAssistant>(_actionSheetAssistant.get())] (WebKit::InteractionInformationAtPosition information) {
    12251227            [assistant interactionDidStartWithPositionInformation:information];
    1226         } forRequest:WebKit::InteractionInformationRequest(WebCore::IntPoint(_lastInteractionLocation))];
     1228        } forRequest:positionInformationRequest];
    12271229    }
    12281230
  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r242833 r242968  
    23772377        if (hitNode && hitNode->renderer()) {
    23782378            RenderObject* renderer = hitNode->renderer();
    2379             m_page->focusController().setFocusedFrame(result.innerNodeFrame());
     2379            if (!request.readonly)
     2380                m_page->focusController().setFocusedFrame(result.innerNodeFrame());
    23802381            info.bounds = renderer->absoluteBoundingBoxRect(true);
    23812382            // We don't want to select blocks that are larger than 97% of the visible area of the document.
Note: See TracChangeset for help on using the changeset viewer.