Changeset 247398 in webkit


Ignore:
Timestamp:
Jul 12, 2019 2:35:08 PM (5 years ago)
Author:
Simon Fraser
Message:

[iOS WK2] Can't place caret or select in content that overflows a contenteditable element
​https://bugs.webkit.org/show_bug.cgi?id=199741
rdar://problem/50545233

Reviewed by Wenson Hsieh.
Source/WebCore:

Various code paths for editing used renderer->absoluteBoundingBoxRect(), which is the border
box of the element (or a set of line boxes for inline elements) converted to absolute
coordinates. This excludes overflow content, but contenteditable needs to be able to
place the caret in overflow content, and allow selection rects to be in the overflow area
(if the element has visible overflow).

Try to clean this up by adding some static helpers on WebPage for accessing the relevant
rects, and use them in code call from visiblePositionInFocusedNodeForPoint(), and
code that is input to selectionClipRect.

This changes selectionClipRect to use the padding box (excluding borders), which is a progression.

Tests: editing/caret/ios/caret-in-overflow-area.html

editing/selection/ios/place-selection-in-overflow-area.html
editing/selection/ios/selection-extends-into-overflow-area.html

  • editing/FrameSelection.cpp:

(WebCore::DragCaretController::editableElementRectInRootViewCoordinates const):

Source/WebKit:

Various code paths for editing used renderer->absoluteBoundingBoxRect(), which is the border
box of the element (or a set of line boxes for inline elements) converted to absolute
coordinates. This excludes overflow content, but contenteditable needs to be able to
place the caret in overflow content, and allow selection rects to be in the overflow area
(if the element has visible overflow).

Try to clean this up by adding some static helpers on WebPage for accessing the relevant
rects, and use them in code call from visiblePositionInFocusedNodeForPoint(), and
code that is input to selectionClipRect.

This changes selectionClipRect to use the padding box (excluding borders), which is a progression.

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

(WebKit::WebPage::platformEditorState const):
(WebKit::elementBoundsInFrame):
(WebKit::constrainPoint):
(WebKit::WebPage::rootViewBoundsForElement):
(WebKit::WebPage::absoluteInteractionBoundsForElement):
(WebKit::WebPage::rootViewInteractionBoundsForElement):
(WebKit::WebPage::dispatchSyntheticMouseEventsForSelectionGesture):
(WebKit::WebPage::getFocusedElementInformation):
(WebKit::innerFrameQuad): Deleted.
(WebKit::elementRectInRootViewCoordinates): Deleted.

LayoutTests:

Re-enable editing/caret/ios, fixing the result of emoji.html which for some reason was
checked in as an html file (the test still fails).

  • editing/caret/ios/caret-in-overflow-area-expected.txt: Added.
  • editing/caret/ios/caret-in-overflow-area.html: Added.
  • editing/caret/ios/emoji-expected.txt: Renamed from LayoutTests/editing/caret/ios/emoji-expected.html.
  • editing/caret/ios/fixed-caret-position-after-scroll-expected.txt:
  • editing/caret/ios/fixed-caret-position-after-scroll.html:
  • editing/selection/ios/place-selection-in-overflow-area-expected.txt: Added.
  • editing/selection/ios/place-selection-in-overflow-area.html: Added.
  • editing/selection/ios/selection-extends-into-overflow-area-expected.txt: Added.
  • editing/selection/ios/selection-extends-into-overflow-area.html: Added.
  • platform/ios-wk2/TestExpectations:
Location:
trunk
Files:
6 added
10 edited
1 moved

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r247391 r247398  
    169169        https://bugs.webkit.org/show_bug.cgi?id=199644
    170170        https://trac.webkit.org/changeset/247314
     171
     1722019-07-11  Simon Fraser  <simon.fraser@apple.com>
     173
     174        [iOS WK2] Can't place caret or select in content that overflows a contenteditable element
     175        https://bugs.webkit.org/show_bug.cgi?id=199741
     176        rdar://problem/50545233
     177
     178        Reviewed by Wenson Hsieh.
     179       
     180        Re-enable editing/caret/ios, fixing the result of emoji.html which for some reason was
     181        checked in as an html file (the test still fails).
     182
     183        * editing/caret/ios/caret-in-overflow-area-expected.txt: Added.
     184        * editing/caret/ios/caret-in-overflow-area.html: Added.
     185        * editing/caret/ios/emoji-expected.txt: Renamed from LayoutTests/editing/caret/ios/emoji-expected.html.
     186        * editing/caret/ios/fixed-caret-position-after-scroll-expected.txt:
     187        * editing/caret/ios/fixed-caret-position-after-scroll.html:
     188        * editing/selection/ios/place-selection-in-overflow-area-expected.txt: Added.
     189        * editing/selection/ios/place-selection-in-overflow-area.html: Added.
     190        * editing/selection/ios/selection-extends-into-overflow-area-expected.txt: Added.
     191        * editing/selection/ios/selection-extends-into-overflow-area.html: Added.
     192        * platform/ios-wk2/TestExpectations:
    171193
    1721942019-07-11  Ryan Haddad  <ryanhaddad@apple.com>

  • trunk/LayoutTests/editing/caret/ios/fixed-caret-position-after-scroll-expected.txt

    r221064 r247398  
    44PASS initialCaretRect.height is 15
    55PASS finalCaretRect.left is 6
    6 PASS finalCaretRect.top is 5021
     6PASS finalCaretRect.top is 4977
    77PASS finalCaretRect.width is 3
    88PASS finalCaretRect.height is 15

  • trunk/LayoutTests/editing/caret/ios/fixed-caret-position-after-scroll.html

    r235893 r247398  
    5252        finalCaretRect = rect;
    5353        shouldBe("finalCaretRect.left", "6");
    54         shouldBe("finalCaretRect.top", "5021");
     54        shouldBe("finalCaretRect.top", "4977");
    5555        shouldBe("finalCaretRect.width", "3");
    5656        shouldBe("finalCaretRect.height", "15");

  • trunk/LayoutTests/platform/ios-wk2/TestExpectations

    r247371 r247398  
    1919scrollingcoordinator/mac [ Skip ]
    2020fast/web-share [ Pass ]
     21editing/caret/ios [ Pass ]
    2122editing/find [ Pass ]
    2223editing/input/ios [ Pass ]
    … …  
    695696editing/undo/undo-misspellings.html [ Failure ]
    696697editing/undo/undo-typing-001.html [ Failure ]
     698editing/caret/ios/emoji.html [ Failure ]
    697699
    698700# Editing tests that time out:

  • trunk/Source/WebCore/ChangeLog

    r247397 r247398  
    498498
    499499        * platform/graphics/cocoa/IOSurface.h:
     500
     5012019-07-11  Simon Fraser  <simon.fraser@apple.com>
     502
     503        [iOS WK2] Can't place caret or select in content that overflows a contenteditable element
     504        https://bugs.webkit.org/show_bug.cgi?id=199741
     505        rdar://problem/50545233
     506
     507        Reviewed by Wenson Hsieh.
     508
     509        Various code paths for editing used renderer->absoluteBoundingBoxRect(), which is the border
     510        box of the element (or a set of line boxes for inline elements) converted to absolute
     511        coordinates. This excludes overflow content, but contenteditable needs to be able to
     512        place the caret in overflow content, and allow selection rects to be in the overflow area
     513        (if the element has visible overflow).
     514
     515        Try to clean this up by adding some static helpers on WebPage for accessing the relevant
     516        rects, and use them in code call from visiblePositionInFocusedNodeForPoint(), and
     517        code that is input to selectionClipRect.
     518       
     519        This changes selectionClipRect to use the padding box (excluding borders), which is a progression.
     520
     521        Tests: editing/caret/ios/caret-in-overflow-area.html
     522               editing/selection/ios/place-selection-in-overflow-area.html
     523               editing/selection/ios/selection-extends-into-overflow-area.html
     524
     525        * editing/FrameSelection.cpp:
     526        (WebCore::DragCaretController::editableElementRectInRootViewCoordinates const):
    500527
    5015282019-07-11  Jonathan Bedard  <jbedard@apple.com>

  • trunk/Source/WebCore/editing/FrameSelection.cpp

    r246948 r247398  
    134134
    135135    if (auto* view = editableContainer->document().view())
    136         return view->contentsToRootView(renderer->absoluteBoundingBoxRect());
     136        return view->contentsToRootView(renderer->absoluteBoundingBoxRect()); // FIXME: Wrong for elements with visible layout overflow.
    137137
    138138    return { };

  • trunk/Source/WebKit/ChangeLog

    r247396 r247398  
    208208        * UIProcess/ios/WebPageProxyIOS.mm:
    209209        (WebKit::desktopClassBrowsingRecommendedForRequest):
     210
     2112019-07-11  Simon Fraser  <simon.fraser@apple.com>
     212
     213        [iOS WK2] Can't place caret or select in content that overflows a contenteditable element
     214        https://bugs.webkit.org/show_bug.cgi?id=199741
     215        rdar://problem/50545233
     216
     217        Reviewed by Wenson Hsieh.
     218
     219        Various code paths for editing used renderer->absoluteBoundingBoxRect(), which is the border
     220        box of the element (or a set of line boxes for inline elements) converted to absolute
     221        coordinates. This excludes overflow content, but contenteditable needs to be able to
     222        place the caret in overflow content, and allow selection rects to be in the overflow area
     223        (if the element has visible overflow).
     224
     225        Try to clean this up by adding some static helpers on WebPage for accessing the relevant
     226        rects, and use them in code call from visiblePositionInFocusedNodeForPoint(), and
     227        code that is input to selectionClipRect.
     228
     229        This changes selectionClipRect to use the padding box (excluding borders), which is a progression.
     230
     231        * WebProcess/WebPage/WebPage.h:
     232        * WebProcess/WebPage/ios/WebPageIOS.mm:
     233        (WebKit::WebPage::platformEditorState const):
     234        (WebKit::elementBoundsInFrame):
     235        (WebKit::constrainPoint):
     236        (WebKit::WebPage::rootViewBoundsForElement):
     237        (WebKit::WebPage::absoluteInteractionBoundsForElement):
     238        (WebKit::WebPage::rootViewInteractionBoundsForElement):
     239        (WebKit::WebPage::dispatchSyntheticMouseEventsForSelectionGesture):
     240        (WebKit::WebPage::getFocusedElementInformation):
     241        (WebKit::innerFrameQuad): Deleted.
     242        (WebKit::elementRectInRootViewCoordinates): Deleted.
    210243
    2112442019-07-11  Jonathan Bedard  <jbedard@apple.com>

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

    r247367 r247398  
    12031203    bool firstFlushAfterCommit() const { return m_firstFlushAfterCommit; }
    12041204    void setFirstFlushAfterCommit(bool f) { m_firstFlushAfterCommit = f; }
    1205    
     1205
     1206#if PLATFORM(IOS_FAMILY)
     1207    // This excludes layout overflow, includes borders.
     1208    static WebCore::IntRect rootViewBoundsForElement(const WebCore::Element&);
     1209    // These include layout overflow for overflow:visible elements, but exclude borders.
     1210    static WebCore::IntRect absoluteInteractionBoundsForElement(const WebCore::Element&);
     1211    static WebCore::IntRect rootViewInteractionBoundsForElement(const WebCore::Element&);
     1212#endif // PLATFORM(IOS_FAMILY)
     1213
    12061214private:
    12071215    WebPage(WebCore::PageIdentifier, WebPageCreationParameters&&);
    … …  
    12361244
    12371245    static void convertSelectionRectsToRootView(WebCore::FrameView*, Vector<WebCore::SelectionRect>&);
     1246
    12381247    void getFocusedElementInformation(FocusedElementInformation&);
    12391248    void platformInitializeAccessibility();

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

    r247369 r247398  
    265265        if (m_focusedElement && m_focusedElement->renderer()) {
    266266            auto& renderer = *m_focusedElement->renderer();
    267             postLayoutData.focusedElementRect = view->contentsToRootView(renderer.absoluteBoundingBoxRect());
     267            postLayoutData.focusedElementRect = rootViewInteractionBoundsForElement(*m_focusedElement);
    268268            postLayoutData.caretColor = renderer.style().caretColor();
    269269        }
    … …  
    12491249}
    12501250
    1251 static FloatQuad innerFrameQuad(const Frame& frame, const Element& focusedElement)
     1251static IntRect elementBoundsInFrame(const Frame& frame, const Element& focusedElement)
    12521252{
    12531253    frame.document()->updateLayoutIgnorePendingStylesheets();
    1254     RenderElement* renderer = nullptr;
     1254   
    12551255    if (focusedElement.hasTagName(HTMLNames::textareaTag) || focusedElement.hasTagName(HTMLNames::inputTag) || focusedElement.hasTagName(HTMLNames::selectTag))
    1256         renderer = focusedElement.renderer();
    1257     else if (auto* rootEditableElement = focusedElement.rootEditableElement())
    1258         renderer = rootEditableElement->renderer();
    1259    
    1260     if (!renderer)
    1261         return FloatQuad();
    1262 
    1263     auto& style = renderer->style();
    1264     IntRect boundingBox = renderer->absoluteBoundingBoxRect(true /* use transforms*/);
    1265 
    1266     boundingBox.move(style.borderLeftWidth(), style.borderTopWidth());
    1267     boundingBox.setWidth(boundingBox.width() - style.borderLeftWidth() - style.borderRightWidth());
    1268     boundingBox.setHeight(boundingBox.height() - style.borderBottomWidth() - style.borderTopWidth());
    1269 
    1270     return FloatQuad(boundingBox);
     1256        return WebPage::absoluteInteractionBoundsForElement(focusedElement);
     1257
     1258    if (auto* rootEditableElement = focusedElement.rootEditableElement())
     1259        return WebPage::absoluteInteractionBoundsForElement(*rootEditableElement);
     1260
     1261    return { };
    12711262}
    12721263
    … …  
    12751266    ASSERT(&focusedElement.document() == frame.document());
    12761267    const int DEFAULT_CONSTRAIN_INSET = 2;
    1277     IntRect innerFrame = innerFrameQuad(frame, focusedElement).enclosingBoundingBox();
     1268    IntRect innerFrame = elementBoundsInFrame(frame, focusedElement);
    12781269    IntPoint constrainedPoint = point;
    12791270
    … …  
    15691560}
    15701561
    1571 static IntRect elementRectInRootViewCoordinates(const Element& element)
     1562IntRect WebPage::rootViewBoundsForElement(const Element& element)
    15721563{
    15731564    auto* frame = element.document().frame();
    … …  
    15861577}
    15871578
     1579IntRect WebPage::absoluteInteractionBoundsForElement(const Element& element)
     1580{
     1581    auto* frame = element.document().frame();
     1582    if (!frame)
     1583        return { };
     1584
     1585    auto* view = frame->view();
     1586    if (!view)
     1587        return { };
     1588
     1589    auto* renderer = element.renderer();
     1590    if (!renderer)
     1591        return { };
     1592
     1593    if (is<RenderBox>(*renderer)) {
     1594        auto& box = downcast<RenderBox>(*renderer);
     1595
     1596        FloatRect rect;
     1597        // FIXME: want borders or not?
     1598        if (box.style().isOverflowVisible())
     1599            rect = box.layoutOverflowRect();
     1600        else
     1601            rect = box.clientBoxRect();
     1602        return box.localToAbsoluteQuad(rect).enclosingBoundingBox();
     1603    }
     1604
     1605    auto& style = renderer->style();
     1606    FloatRect boundingBox = renderer->absoluteBoundingBoxRect(true /* use transforms*/);
     1607    // This is wrong. It's subtracting borders after converting to absolute coords on something that probably doesn't represent a rectangular element.
     1608    boundingBox.move(style.borderLeftWidth(), style.borderTopWidth());
     1609    boundingBox.setWidth(boundingBox.width() - style.borderLeftWidth() - style.borderRightWidth());
     1610    boundingBox.setHeight(boundingBox.height() - style.borderBottomWidth() - style.borderTopWidth());
     1611    return enclosingIntRect(boundingBox);
     1612}
     1613
     1614IntRect WebPage::rootViewInteractionBoundsForElement(const Element& element)
     1615{
     1616    auto* frame = element.document().frame();
     1617    if (!frame)
     1618        return { };
     1619
     1620    auto* view = frame->view();
     1621    if (!view)
     1622        return { };
     1623
     1624    return view->contentsToRootView(absoluteInteractionBoundsForElement(element));
     1625}
     1626
    15881627void WebPage::clearSelection()
    15891628{
    … …  
    16001639    IntRect focusedElementRect;
    16011640    if (m_focusedElement)
    1602         focusedElementRect = elementRectInRootViewCoordinates(*m_focusedElement);
     1641        focusedElementRect = rootViewInteractionBoundsForElement(*m_focusedElement);
    16031642
    16041643    if (focusedElementRect.isEmpty())
    … …  
    28352874
    28362875    if (auto* renderer = m_focusedElement->renderer()) {
    2837         information.elementRect = elementRectInRootViewCoordinates(*m_focusedElement);
     2876        information.elementRect = rootViewInteractionBoundsForElement(*m_focusedElement);
    28382877        information.nodeFontSize = renderer->style().fontDescription().computedSize();
    28392878
    … …  
    28542893    information.allowsUserScalingIgnoringAlwaysScalable = m_viewportConfiguration.allowsUserScalingIgnoringAlwaysScalable();
    28552894    if (auto* nextElement = nextAssistableElement(m_focusedElement.get(), *m_page, true)) {
    2856         information.nextNodeRect = elementRectInRootViewCoordinates(*nextElement);
     2895        information.nextNodeRect = rootViewBoundsForElement(*nextElement);
    28572896        information.hasNextNode = true;
    28582897    }
    28592898    if (auto* previousElement = nextAssistableElement(m_focusedElement.get(), *m_page, false)) {
    2860         information.previousNodeRect = elementRectInRootViewCoordinates(*previousElement);
     2899        information.previousNodeRect = rootViewBoundsForElement(*previousElement);
    28612900        information.hasPreviousNode = true;
    28622901    }

  • trunk/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm

    r246924 r247398  
    522522    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.querySelector('input').focus()"];
    523523
    524     EXPECT_EQ(10, selectionClipRect.origin.x);
    525     EXPECT_EQ(10, selectionClipRect.origin.y);
    526     EXPECT_EQ(136, selectionClipRect.size.width);
    527     EXPECT_EQ(22, selectionClipRect.size.height);
     524    EXPECT_EQ(11, selectionClipRect.origin.x);
     525    EXPECT_EQ(11, selectionClipRect.origin.y);
     526    EXPECT_EQ(134, selectionClipRect.size.width);
     527    EXPECT_EQ(20, selectionClipRect.size.height);
    528528}
    529529
Note: See TracChangeset for help on using the changeset viewer.