Changeset 244546 in webkit


Ignore:
Timestamp:
Apr 23, 2019 10:10:10 AM (5 years ago)
Author:
rniwa@webkit.org
Message:

[iOS] element.focus() sometimes fails to reveal the focused element when it becomes editable dynamically
https://bugs.webkit.org/show_bug.cgi?id=197188

Reviewed by Wenson Hsieh.

Source/WebCore:

The bug was caused by the scroll-to-reveal code triggered by Element::updateFocusAppearance updating
the scroll position via scrolling tree update in a layer tree commit which happens after
_zoomToRevealFocusedElement in WKContentView had already scrolled the frame view.

To fix this problem, we need to defer the editor state update until the layer commit (see r244494),
and update the scrolling tree before invoking WebPageProxy::editorStateChanged which brings up
the keyboard and scroll-to-reveal the caret in the UI process side.

We also avoid revealing the focus for the second time via Document::scheduleScrollToFocusedElement
in Element::updateFocusAppearance as this timer based scrolling also happens after we had already
revealed the caret in _zoomToRevealFocusedElement. This is a bit hacky but works for most cases since
we wouldn't bring up a keyboard if the focused element was not editable anyway.

Test: editing/selection/ios/scrolling-to-focused-element-inside-iframe.html

  • dom/Element.cpp:

(WebCore::Element::updateFocusAppearance): Avoid scheduling a timer based reavel of the focused element
when we're already revealing the element via selection change.

Source/WebKit:

Commit the scroll tree update before revealing the keyboard via editor state update.

  • UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:

(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):

LayoutTests:

Added a regression test.

  • editing/selection/ios/scrolling-to-focused-element-inside-iframe-expected.txt: Added.
  • editing/selection/ios/scrolling-to-focused-element-inside-iframe.html: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r244544 r244546  
     12019-04-23  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        [iOS] element.focus() sometimes fails to reveal the focused element when it becomes editable dynamically
     4        https://bugs.webkit.org/show_bug.cgi?id=197188
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        Added a regression test.
     9
     10        * editing/selection/ios/scrolling-to-focused-element-inside-iframe-expected.txt: Added.
     11        * editing/selection/ios/scrolling-to-focused-element-inside-iframe.html: Added.
     12
    1132019-04-23  John Wilander  <wilander@apple.com>
    214
  • trunk/Source/WebCore/ChangeLog

    r244545 r244546  
     12019-04-23  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        [iOS] element.focus() sometimes fails to reveal the focused element when it becomes editable dynamically
     4        https://bugs.webkit.org/show_bug.cgi?id=197188
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        The bug was caused by the scroll-to-reveal code triggered by Element::updateFocusAppearance updating
     9        the scroll position via scrolling tree update in a layer tree commit which happens after
     10        _zoomToRevealFocusedElement in WKContentView had already scrolled the frame view.
     11
     12        To fix this problem, we need to defer the editor state update until the layer commit (see r244494),
     13        and update the scrolling tree before invoking WebPageProxy::editorStateChanged which brings up
     14        the keyboard and scroll-to-reveal the caret in the UI process side.
     15
     16        We also avoid revealing the focus for the second time via Document::scheduleScrollToFocusedElement
     17        in Element::updateFocusAppearance as this timer based scrolling also happens after we had already
     18        revealed the caret in _zoomToRevealFocusedElement. This is a bit hacky but works for most cases since
     19        we wouldn't bring up a keyboard if the focused element was not editable anyway.
     20
     21        Test: editing/selection/ios/scrolling-to-focused-element-inside-iframe.html
     22
     23        * dom/Element.cpp:
     24        (WebCore::Element::updateFocusAppearance): Avoid scheduling a timer based reavel of the focused element
     25        when we're already revealing the element via selection change.
     26
    1272019-04-23  Remy Demarest  <rdemarest@apple.com>
    228
  • trunk/Source/WebCore/dom/Element.cpp

    r244440 r244546  
    27862786            frame->selection().setSelection(newSelection, FrameSelection::defaultSetSelectionOptions(), Element::defaultFocusTextStateChangeIntent());
    27872787            frame->selection().revealSelection(revealMode);
     2788            return;
    27882789        }
    27892790    }
  • trunk/Source/WebKit/ChangeLog

    r244545 r244546  
     12019-04-23  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        [iOS] element.focus() sometimes fails to reveal the focused element when it becomes editable dynamically
     4        https://bugs.webkit.org/show_bug.cgi?id=197188
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        Commit the scroll tree update before revealing the keyboard via editor state update.
     9
     10        * UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
     11        (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
     12
    1132019-04-23  Remy Demarest  <rdemarest@apple.com>
    214
  • trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm

    r244291 r244546  
    200200    m_activityStateChangeID = layerTreeTransaction.activityStateChangeID();
    201201
    202     if (layerTreeTransaction.hasEditorState())
    203         m_webPageProxy.editorStateChanged(layerTreeTransaction.editorState());
    204 
    205202    if (m_remoteLayerTreeHost->updateLayerTree(layerTreeTransaction)) {
    206203        if (layerTreeTransaction.transactionID() >= m_transactionIDForUnhidingContent)
     
    255252    didRefreshDisplay();
    256253#endif
     254
     255    if (layerTreeTransaction.hasEditorState())
     256        m_webPageProxy.editorStateChanged(layerTreeTransaction.editorState());
    257257
    258258    if (auto milestones = layerTreeTransaction.newlyReachedPaintingMilestones())
Note: See TracChangeset for help on using the changeset viewer.