⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 278633 in webkit


Ignore:
Timestamp:
Jun 8, 2021, 4:30:21 PM (5 years ago)
Author:
Wenson Hsieh
Message:

[iOS] Safari tab pill should toggle visibility when tapping on article text on adventure.com
​https://bugs.webkit.org/show_bug.cgi?id=226775
rdar://78826820

Reviewed by Tim Horton and Devin Rousso.

Source/WebKit:

Adjust the meaningful click heuristic to account for click event listeners added to the document node. See below
for more details.

Test: fast/events/ios/non-meaningful-click-when-tapping-document.html

  • WebProcess/WebPage/WebPage.h:
  • WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::getPlatformEditorState const):
(WebKit::isProbablyMeaningfulClick):

Remove a check for whether or not the clicked node is an Element; this existed because the call to
rootViewBoundsForElement below takes an Element rather than just a Node; however, this method doesn't do
anything that requires an Element instead of a Node, so we can just remove the check and refactor these static
methods to accept Nodes. This allows us to bail early if the clicked node is *either* the body or the document
node, instead of just the body.

(WebKit::WebPage::insertDroppedImagePlaceholders):
(WebKit::elementBoundsInFrame):
(WebKit::WebPage::rootViewBounds):
(WebKit::WebPage::absoluteInteractionBounds):
(WebKit::WebPage::rootViewInteractionBounds):

Drive-by refactoring: drop the -ForElement suffixes on these helper methods, and additionally make them accept
a Node instead of requiring an Element. This allows us to remove the is<Element>() check from the meaningful
click heuristic above.

Also deploy RefPtr in a few more places.

(WebKit::WebPage::dispatchSyntheticMouseEventsForSelectionGesture):
(WebKit::WebPage::focusedElementInformation):
(WebKit::WebPage::rootViewBoundsForElement): Deleted.
(WebKit::WebPage::absoluteInteractionBoundsForElement): Deleted.
(WebKit::WebPage::rootViewInteractionBoundsForElement): Deleted.

LayoutTests:

  • fast/events/ios/non-meaningful-click-when-tapping-document-expected.txt: Added.
  • fast/events/ios/non-meaningful-click-when-tapping-document.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r278632 r278633  
     12021-06-08  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] Safari tab pill should toggle visibility when tapping on article text on adventure.com
     4        https://bugs.webkit.org/show_bug.cgi?id=226775
     5        rdar://78826820
     6
     7        Reviewed by Tim Horton and Devin Rousso.
     8
     9        * fast/events/ios/non-meaningful-click-when-tapping-document-expected.txt: Added.
     10        * fast/events/ios/non-meaningful-click-when-tapping-document.html: Added.
     11
    1122021-06-08  Diego Pino Garcia  <dpino@igalia.com>
    213

  • trunk/Source/WebKit/ChangeLog

    r278630 r278633  
     12021-06-08  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] Safari tab pill should toggle visibility when tapping on article text on adventure.com
     4        https://bugs.webkit.org/show_bug.cgi?id=226775
     5        rdar://78826820
     6
     7        Reviewed by Tim Horton and Devin Rousso.
     8
     9        Adjust the meaningful click heuristic to account for click event listeners added to the document node. See below
     10        for more details.
     11
     12        Test: fast/events/ios/non-meaningful-click-when-tapping-document.html
     13
     14        * WebProcess/WebPage/WebPage.h:
     15        * WebProcess/WebPage/ios/WebPageIOS.mm:
     16        (WebKit::WebPage::getPlatformEditorState const):
     17        (WebKit::isProbablyMeaningfulClick):
     18
     19        Remove a check for whether or not the clicked node is an Element; this existed because the call to
     20        `rootViewBoundsForElement` below takes an Element rather than just a Node; however, this method doesn't do
     21        anything that requires an Element instead of a Node, so we can just remove the check and refactor these static
     22        methods to accept Nodes. This allows us to bail early if the clicked node is *either* the body or the document
     23        node, instead of just the body.
     24
     25        (WebKit::WebPage::insertDroppedImagePlaceholders):
     26        (WebKit::elementBoundsInFrame):
     27        (WebKit::WebPage::rootViewBounds):
     28        (WebKit::WebPage::absoluteInteractionBounds):
     29        (WebKit::WebPage::rootViewInteractionBounds):
     30
     31        Drive-by refactoring: drop the `-ForElement` suffixes on these helper methods, and additionally make them accept
     32        a Node instead of requiring an Element. This allows us to remove the `is<Element>()` check from the meaningful
     33        click heuristic above.
     34
     35        Also deploy RefPtr in a few more places.
     36
     37        (WebKit::WebPage::dispatchSyntheticMouseEventsForSelectionGesture):
     38        (WebKit::WebPage::focusedElementInformation):
     39        (WebKit::WebPage::rootViewBoundsForElement): Deleted.
     40        (WebKit::WebPage::absoluteInteractionBoundsForElement): Deleted.
     41        (WebKit::WebPage::rootViewInteractionBoundsForElement): Deleted.
     42
    1432021-06-08  Devin Rousso  <drousso@apple.com>
    244

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

    r278575 r278633  
    13571357#if PLATFORM(IOS_FAMILY)
    13581358    // This excludes layout overflow, includes borders.
    1359     static WebCore::IntRect rootViewBoundsForElement(const WebCore::Element&);
     1359    static WebCore::IntRect rootViewBounds(const WebCore::Node&);
    13601360    // These include layout overflow for overflow:visible elements, but exclude borders.
    1361     static WebCore::IntRect absoluteInteractionBoundsForElement(const WebCore::Element&);
    1362     static WebCore::IntRect rootViewInteractionBoundsForElement(const WebCore::Element&);
     1361    static WebCore::IntRect absoluteInteractionBounds(const WebCore::Node&);
     1362    static WebCore::IntRect rootViewInteractionBounds(const WebCore::Node&);
    13631363
    13641364    InteractionInformationAtPosition positionInformation(const InteractionInformationRequest&);

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

    r278542 r278633  
    345345
    346346        if (auto editableRootOrFormControl = makeRefPtr(enclosingTextFormControl(selection.start()) ?: selection.rootEditableElement())) {
    347             postLayoutData.selectionClipRect = rootViewInteractionBoundsForElement(*editableRootOrFormControl);
     347            postLayoutData.selectionClipRect = rootViewInteractionBounds(*editableRootOrFormControl);
    348348            postLayoutData.editableRootIsTransparentOrFullyClipped = result.isContentEditable && isTransparentOrFullyClipped(*editableRootOrFormControl);
    349349        }
    … …  
    840840{
    841841    auto frame = makeRefPtr(clickNode.document().frame());
    842     if (!is<Element>(clickNode) || !clickNode.isConnected() || !frame)
     842    if (!frame || !clickNode.isConnected())
    843843        return true;
    844844
    845     if (is<HTMLBodyElement>(clickNode))
     845    if (is<HTMLBodyElement>(clickNode) || is<Document>(clickNode) || clickNode.document().documentElement() == &clickNode)
    846846        return false;
    847847
    848848    if (auto view = makeRefPtr(frame->mainFrame().view())) {
    849         auto elementBounds = WebPage::rootViewBoundsForElement(downcast<Element>(clickNode));
     849        auto elementBounds = WebPage::rootViewInteractionBounds(clickNode);
    850850        auto unobscuredRect = view->unobscuredContentRect();
    851851        if (elementBounds.width() >= unobscuredRect.width() / 2 && elementBounds.height() >= unobscuredRect.height() / 2)
    … …  
    10061006    m_page->dragController().insertDroppedImagePlaceholdersAtCaret(imageSizes);
    10071007    auto placeholderRects = m_page->dragController().droppedImagePlaceholders().map([&] (auto& element) {
    1008         return rootViewBoundsForElement(element);
     1008        return rootViewBounds(element);
    10091009    });
    10101010
    … …  
    13611361   
    13621362    if (focusedElement.hasTagName(HTMLNames::textareaTag) || focusedElement.hasTagName(HTMLNames::inputTag) || focusedElement.hasTagName(HTMLNames::selectTag))
    1363         return WebPage::absoluteInteractionBoundsForElement(focusedElement);
     1363        return WebPage::absoluteInteractionBounds(focusedElement);
    13641364
    13651365    if (auto* rootEditableElement = focusedElement.rootEditableElement())
    1366         return WebPage::absoluteInteractionBoundsForElement(*rootEditableElement);
     1366        return WebPage::absoluteInteractionBounds(*rootEditableElement);
    13671367
    13681368    return { };
    … …  
    16791679}
    16801680
    1681 IntRect WebPage::rootViewBoundsForElement(const Element& element)
    1682 {
    1683     auto* frame = element.document().frame();
     1681IntRect WebPage::rootViewBounds(const Node& node)
     1682{
     1683    auto frame = makeRefPtr(node.document().frame());
    16841684    if (!frame)
    16851685        return { };
    16861686
    1687     auto* view = frame->view();
     1687    auto view = makeRefPtr(frame->view());
    16881688    if (!view)
    16891689        return { };
    16901690
    1691     auto* renderer = element.renderer();
     1691    auto* renderer = node.renderer();
    16921692    if (!renderer)
    16931693        return { };
    … …  
    16961696}
    16971697
    1698 IntRect WebPage::absoluteInteractionBoundsForElement(const Element& element)
    1699 {
    1700     auto* frame = element.document().frame();
     1698IntRect WebPage::absoluteInteractionBounds(const Node& node)
     1699{
     1700    auto frame = makeRefPtr(node.document().frame());
    17011701    if (!frame)
    17021702        return { };
    17031703
    1704     auto* view = frame->view();
     1704    auto view = makeRefPtr(frame->view());
    17051705    if (!view)
    17061706        return { };
    17071707
    1708     auto* renderer = element.renderer();
     1708    auto* renderer = node.renderer();
    17091709    if (!renderer)
    17101710        return { };
    … …  
    17311731}
    17321732
    1733 IntRect WebPage::rootViewInteractionBoundsForElement(const Element& element)
    1734 {
    1735     auto* frame = element.document().frame();
     1733IntRect WebPage::rootViewInteractionBounds(const Node& node)
     1734{
     1735    auto frame = makeRefPtr(node.document().frame());
    17361736    if (!frame)
    17371737        return { };
    17381738
    1739     auto* view = frame->view();
     1739    auto view = makeRefPtr(frame->view());
    17401740    if (!view)
    17411741        return { };
    17421742
    1743     return view->contentsToRootView(absoluteInteractionBoundsForElement(element));
     1743    return view->contentsToRootView(absoluteInteractionBounds(node));
    17441744}
    17451745
    … …  
    17581758    IntRect focusedElementRect;
    17591759    if (m_focusedElement)
    1760         focusedElementRect = rootViewInteractionBoundsForElement(*m_focusedElement);
     1760        focusedElementRect = rootViewInteractionBounds(*m_focusedElement);
    17611761
    17621762    if (focusedElementRect.isEmpty())
    … …  
    32433243
    32443244    if (auto* renderer = focusedElement->renderer()) {
    3245         information.interactionRect = rootViewInteractionBoundsForElement(*focusedElement);
     3245        information.interactionRect = rootViewInteractionBounds(*focusedElement);
    32463246        information.nodeFontSize = renderer->style().fontDescription().computedSize();
    32473247
    … …  
    32703270    information.allowsUserScalingIgnoringAlwaysScalable = m_viewportConfiguration.allowsUserScalingIgnoringAlwaysScalable();
    32713271    if (auto* nextElement = nextAssistableElement(focusedElement.get(), *m_page, true)) {
    3272         information.nextNodeRect = rootViewBoundsForElement(*nextElement);
     3272        information.nextNodeRect = rootViewBounds(*nextElement);
    32733273        information.hasNextNode = true;
    32743274    }
    32753275    if (auto* previousElement = nextAssistableElement(focusedElement.get(), *m_page, false)) {
    3276         information.previousNodeRect = rootViewBoundsForElement(*previousElement);
     3276        information.previousNodeRect = rootViewBounds(*previousElement);
    32773277        information.hasPreviousNode = true;
    32783278    }
Note: See TracChangeset for help on using the changeset viewer.