Changeset 225715 in webkit


Ignore:
Timestamp:
Dec 8, 2017, 4:00:14 PM (7 years ago)
Author:
Simon Fraser
Message:

Wrong caret position for input field inside a fixed position parent on iOS 11
​https://bugs.webkit.org/show_bug.cgi?id=176896
rdar://problem/33726145

Reviewed by Tim Horton.
Source/WebCore:

In r219668 I added code to compute a layout viewport rect in the web process, so that
after programmatic scrolling, getBoundingClientRect() would return the correct values.
However, that computation sometimes used a different visual viewport than the UI process,
resulting in a different layout viewport being set. This would happen when the keyboard
was visible, and the combination of this and zooming when focusing an input would result
in a state where the scrolling tree contained notes computed with the bad layout viewport.
This could cause apparently offset fixed elements, and bad caret positioning if those fixed
elements contained the focused input.

Fix by passing to the web process the same visual viewport rect that the UI process is using,
namely "unobscuredContentRectRespectingInputViewBounds". This was already being set in
VisibleContentRectUpdateInfo but wasn't encoded/decoded, so fix that. Set it as an optional<>
on FrameView when different from the normal visual viewport, and return it from
visualViewportRect().

Some other minor logging changes.

Test: fast/visual-viewport/ios/caret-after-focus-in-fixed.html

  • page/FrameView.cpp:

(WebCore::FrameView::setVisualViewportOverrideRect):
(WebCore::FrameView::updateLayoutViewport):
(WebCore::FrameView::visualViewportRect const):

  • page/FrameView.h:
  • page/scrolling/mac/ScrollingTreeFixedNode.mm:

(WebCore::ScrollingTreeFixedNode::updateLayersAfterAncestorChange):

Source/WebKit:

In r219668 I added code to compute a layout viewport rect in the web process, so that
after programmatic scrolling, getBoundingClientRect() would return the correct values.
However, that computation sometimes used a different visual viewport than the UI process,
resulting in a different layout viewport being set. This would happen when the keyboard
was visible, and the combination of this and zooming when focusing an input would result
in a state where the scrolling tree contained notes computed with the bad layout viewport.
This could cause apparently offset fixed elements, and bad caret positioning if those fixed
elements contained the focused input.

Fix by passing to the web process the same visual viewport rect that the UI process is using,
namely "unobscuredContentRectRespectingInputViewBounds". This was already being set in
VisibleContentRectUpdateInfo but wasn't encoded/decoded, so fix that. Set it as an optional<>
on FrameView when different from the normal visual viewport, and return it from
visualViewportRect().

Some other minor logging changes.

  • Shared/VisibleContentRectUpdateInfo.cpp:

(WebKit::VisibleContentRectUpdateInfo::encode const):
(WebKit::VisibleContentRectUpdateInfo::decode):
(WebKit::operator<<):

  • Shared/VisibleContentRectUpdateInfo.h:

(WebKit::VisibleContentRectUpdateInfo::VisibleContentRectUpdateInfo):

  • WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::updateVisibleContentRects):

LayoutTests:

Test that focuses an input inside position:fixed, then moves focus to the next
input. This was the most reliable way I could find of triggering the bug.
The test dumps the caret rect.

  • fast/visual-viewport/ios/caret-after-focus-in-fixed-expected.txt: Added.
  • fast/visual-viewport/ios/caret-after-focus-in-fixed.html: Added.
Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r225714 r225715  
     12017-12-07  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Wrong caret position for input field inside a fixed position parent on iOS 11
     4        https://bugs.webkit.org/show_bug.cgi?id=176896
     5        rdar://problem/33726145
     6
     7        Reviewed by Tim Horton.
     8       
     9        Test that focuses an input inside position:fixed, then moves focus to the next
     10        input. This was the most reliable way I could find of triggering the bug.
     11        The test dumps the caret rect.
     12
     13        * fast/visual-viewport/ios/caret-after-focus-in-fixed-expected.txt: Added.
     14        * fast/visual-viewport/ios/caret-after-focus-in-fixed.html: Added.
     15
    1162017-12-06  Simon Fraser  <simon.fraser@apple.com>
    217

  • trunk/Source/WebCore/ChangeLog

    r225712 r225715  
     12017-12-07  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Wrong caret position for input field inside a fixed position parent on iOS 11
     4        https://bugs.webkit.org/show_bug.cgi?id=176896
     5        rdar://problem/33726145
     6
     7        Reviewed by Tim Horton.
     8
     9        In r219668 I added code to compute a layout viewport rect in the web process, so that
     10        after programmatic scrolling, getBoundingClientRect() would return the correct values.
     11        However, that computation sometimes used a different visual viewport than the UI process,
     12        resulting in a different layout viewport being set. This would happen when the keyboard
     13        was visible, and the combination of this and zooming when focusing an input would result
     14        in a state where the scrolling tree contained notes computed with the bad layout viewport.
     15        This could cause apparently offset fixed elements, and bad caret positioning if those fixed
     16        elements contained the focused input.
     17
     18        Fix by passing to the web process the same visual viewport rect that the UI process is using,
     19        namely "unobscuredContentRectRespectingInputViewBounds". This was already being set in
     20        VisibleContentRectUpdateInfo but wasn't encoded/decoded, so fix that. Set it as an optional<>
     21        on FrameView when different from the normal visual viewport, and return it from
     22        visualViewportRect().
     23
     24        Some other minor logging changes.
     25
     26        Test: fast/visual-viewport/ios/caret-after-focus-in-fixed.html
     27
     28        * page/FrameView.cpp:
     29        (WebCore::FrameView::setVisualViewportOverrideRect):
     30        (WebCore::FrameView::updateLayoutViewport):
     31        (WebCore::FrameView::visualViewportRect const):
     32        * page/FrameView.h:
     33        * page/scrolling/mac/ScrollingTreeFixedNode.mm:
     34        (WebCore::ScrollingTreeFixedNode::updateLayersAfterAncestorChange):
     35
    1362017-12-08  Zalan Bujtas  <zalan@apple.com>
    237

  • trunk/Source/WebCore/page/FrameView.cpp

    r225512 r225715  
    16371637}
    16381638
     1639void FrameView::setVisualViewportOverrideRect(std::optional<LayoutRect> rect)
     1640{
     1641    m_visualViewportOverrideRect = rect;
     1642}
     1643
    16391644LayoutSize FrameView::baseLayoutViewportSize() const
    16401645{
    … …  
    16561661    LOG_WITH_STREAM(Scrolling, stream << "\nFrameView " << this << " updateLayoutViewport() totalContentSize " << totalContentsSize() << " unscaledDocumentRect " << (renderView() ? renderView()->unscaledDocumentRect() : IntRect()) << " header height " << headerHeight() << " footer height " << footerHeight() << " fixed behavior " << scrollBehaviorForFixedElements());
    16571662    LOG_WITH_STREAM(Scrolling, stream << "layoutViewport: " << layoutViewport);
    1658     LOG_WITH_STREAM(Scrolling, stream << "visualViewport: " << visualViewportRect());
     1663    LOG_WITH_STREAM(Scrolling, stream << "visualViewport: " << visualViewportRect() << " (is override " << (bool)m_visualViewportOverrideRect << ")");
    16591664    LOG_WITH_STREAM(Scrolling, stream << "stable origins: min: " << minStableLayoutViewportOrigin() << " max: "<< maxStableLayoutViewportOrigin());
    16601665   
    16611666    if (m_layoutViewportOverrideRect) {
    16621667        if (m_inProgrammaticScroll) {
     1668            LOG_WITH_STREAM(Scrolling, stream << "computing new override layout viewport because of programmatic scrolling");
    16631669            LayoutPoint newOrigin = computeLayoutViewportOrigin(visualViewportRect(), minStableLayoutViewportOrigin(), maxStableLayoutViewportOrigin(), layoutViewport, StickToDocumentBounds);
    16641670            setLayoutViewportOverrideRect(LayoutRect(newOrigin, m_layoutViewportOverrideRect.value().size()));
    … …  
    17271733LayoutRect FrameView::visualViewportRect() const
    17281734{
     1735    if (m_visualViewportOverrideRect)
     1736        return m_visualViewportOverrideRect.value();
     1737
    17291738    FloatRect visibleContentRect = this->visibleContentRect(LegacyIOSDocumentVisibleRect);
    17301739    return visibleDocumentRect(visibleContentRect, headerHeight(), footerHeight(), totalContentsSize(), frameScaleFactor());

  • trunk/Source/WebCore/page/FrameView.h

    r225052 r225715  
    250250    // Used with delegated scrolling (i.e. iOS).
    251251    WEBCORE_EXPORT void setLayoutViewportOverrideRect(std::optional<LayoutRect>, TriggerLayoutOrNot = TriggerLayoutOrNot::Yes);
     252
     253    WEBCORE_EXPORT void setVisualViewportOverrideRect(std::optional<LayoutRect>);
    252254
    253255    // These are in document coordinates, unaffected by page scale (but affected by zooming).
    … …  
    823825    LayoutPoint m_layoutViewportOrigin;
    824826    std::optional<LayoutRect> m_layoutViewportOverrideRect;
     827    std::optional<LayoutRect> m_visualViewportOverrideRect; // Used when the iOS keyboard is showing.
    825828
    826829    RefPtr<Node> m_nodeToDraw;

  • trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFixedNode.mm

    r223476 r225715  
    7676    FloatPoint layerPosition = m_constraints.layerPositionForViewportRect(fixedPositionRect);
    7777
    78     LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeFixedNode " << scrollingNodeID() << " updateLayersAfterAncestorChange: new viewport " << fixedPositionRect << " viewportRectAtLastLayout " << m_constraints.viewportRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " new offset from bottom " << (fixedPositionRect.maxY() - layerPosition.y()));
     78    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeFixedNode " << scrollingNodeID() << " updateLayersAfterAncestorChange: new viewport " << fixedPositionRect << " viewportRectAtLastLayout " << m_constraints.viewportRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " new offset from top " << (fixedPositionRect.y() - layerPosition.y()));
    7979
    8080    layerPosition -= cumulativeDelta;

  • trunk/Source/WebKit/ChangeLog

    r225714 r225715  
     12017-12-07  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Wrong caret position for input field inside a fixed position parent on iOS 11
     4        https://bugs.webkit.org/show_bug.cgi?id=176896
     5        rdar://problem/33726145
     6
     7        Reviewed by Tim Horton.
     8
     9        In r219668 I added code to compute a layout viewport rect in the web process, so that
     10        after programmatic scrolling, getBoundingClientRect() would return the correct values.
     11        However, that computation sometimes used a different visual viewport than the UI process,
     12        resulting in a different layout viewport being set. This would happen when the keyboard
     13        was visible, and the combination of this and zooming when focusing an input would result
     14        in a state where the scrolling tree contained notes computed with the bad layout viewport.
     15        This could cause apparently offset fixed elements, and bad caret positioning if those fixed
     16        elements contained the focused input.
     17
     18        Fix by passing to the web process the same visual viewport rect that the UI process is using,
     19        namely "unobscuredContentRectRespectingInputViewBounds". This was already being set in
     20        VisibleContentRectUpdateInfo but wasn't encoded/decoded, so fix that. Set it as an optional<>
     21        on FrameView when different from the normal visual viewport, and return it from
     22        visualViewportRect().
     23
     24        Some other minor logging changes.
     25
     26        * Shared/VisibleContentRectUpdateInfo.cpp:
     27        (WebKit::VisibleContentRectUpdateInfo::encode const):
     28        (WebKit::VisibleContentRectUpdateInfo::decode):
     29        (WebKit::operator<<):
     30        * Shared/VisibleContentRectUpdateInfo.h:
     31        (WebKit::VisibleContentRectUpdateInfo::VisibleContentRectUpdateInfo):
     32        * WebProcess/WebPage/ios/WebPageIOS.mm:
     33        (WebKit::WebPage::updateVisibleContentRects):
     34
    1352017-12-06  Simon Fraser  <simon.fraser@apple.com>
    236

  • trunk/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp

    r220503 r225715  
    3939    encoder << m_exposedContentRect;
    4040    encoder << m_unobscuredContentRect;
     41    encoder << m_unobscuredContentRectRespectingInputViewBounds;
    4142    encoder << m_unobscuredRectInScrollViewCoordinates;
    4243    encoder << m_customFixedPositionRect;
    … …  
    6162        return false;
    6263    if (!decoder.decode(result.m_unobscuredContentRect))
     64        return false;
     65    if (!decoder.decode(result.m_unobscuredContentRectRespectingInputViewBounds))
    6366        return false;
    6467    if (!decoder.decode(result.m_unobscuredRectInScrollViewCoordinates))
    … …  
    113116    ts.dumpProperty("exposedContentRect", info.exposedContentRect());
    114117    ts.dumpProperty("unobscuredContentRect", info.unobscuredContentRect());
     118    ts.dumpProperty("unobscuredContentRectRespectingInputViewBounds", info.unobscuredContentRectRespectingInputViewBounds());
    115119    ts.dumpProperty("unobscuredRectInScrollViewCoordinates", info.unobscuredRectInScrollViewCoordinates());
    116     ts.dumpProperty("unobscuredContentRectRespectingInputViewBounds", info.unobscuredContentRectRespectingInputViewBounds());
    117120    ts.dumpProperty("customFixedPositionRect", info.customFixedPositionRect());
    118121    ts.dumpProperty("obscuredInsets", info.obscuredInsets());

  • trunk/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h

    r220503 r225715  
    4949        : m_exposedContentRect(exposedContentRect)
    5050        , m_unobscuredContentRect(unobscuredContentRect)
     51        , m_unobscuredContentRectRespectingInputViewBounds(unobscuredContentRectRespectingInputViewBounds)
    5152        , m_unobscuredRectInScrollViewCoordinates(unobscuredRectInScrollViewCoordinates)
    52         , m_unobscuredContentRectRespectingInputViewBounds(unobscuredContentRectRespectingInputViewBounds)
    5353        , m_customFixedPositionRect(customFixedPositionRect)
    5454        , m_obscuredInsets(obscuredInsets)
    … …  
    9898    WebCore::FloatRect m_exposedContentRect;
    9999    WebCore::FloatRect m_unobscuredContentRect;
     100    WebCore::FloatRect m_unobscuredContentRectRespectingInputViewBounds;
    100101    WebCore::FloatRect m_unobscuredRectInScrollViewCoordinates;
    101     WebCore::FloatRect m_unobscuredContentRectRespectingInputViewBounds;
    102102    WebCore::FloatRect m_customFixedPositionRect; // When visual viewports are enabled, this is the layout viewport.
    103103    WebCore::FloatBoxExtent m_obscuredInsets;

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

    r225449 r225715  
    28452845    if (m_isInStableState) {
    28462846        if (frameView.frame().settings().visualViewportEnabled()) {
     2847            if (visibleContentRectUpdateInfo.unobscuredContentRect() != visibleContentRectUpdateInfo.unobscuredContentRectRespectingInputViewBounds())
     2848                frameView.setVisualViewportOverrideRect(LayoutRect(visibleContentRectUpdateInfo.unobscuredContentRectRespectingInputViewBounds()));
     2849            else
     2850                frameView.setVisualViewportOverrideRect(std::nullopt);
     2851
     2852            LOG_WITH_STREAM(VisibleRects, stream << "WebPage::updateVisibleContentRects - setLayoutViewportOverrideRect " << visibleContentRectUpdateInfo.customFixedPositionRect());
    28472853            frameView.setLayoutViewportOverrideRect(LayoutRect(visibleContentRectUpdateInfo.customFixedPositionRect()));
    28482854            if (selectionIsInsideFixedPositionContainer(frame)) {
Note: See TracChangeset for help on using the changeset viewer.