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

Changeset 248988 in webkit


Ignore:
Timestamp:
Aug 21, 2019, 6:25:48 PM (7 years ago)
Author:
Kocsen Chung
Message:

Cherry-pick r248974. rdar://problem/54579634

[Mail] Tapping top of message scrolls back to copied text instead of top of the message
​https://bugs.webkit.org/show_bug.cgi?id=200999
<rdar://problem/54564878>

Reviewed by Wenson Hsieh.

Source/WebCore:

Test: editing/selection/ios/change-selection-by-tapping-with-existing-selection.html

  • page/EditorClient.h: (WebCore::EditorClient::shouldAllowSingleClickToChangeSelection const):
  • page/EventHandler.cpp: (WebCore::EventHandler::handleMousePressEventSingleClick): Instead of encoding platform behaviors in EventHandler, defer to EditorClient.

Source/WebKit:

In the case where you have a WebCore selection but are not first responder,
when you tap the WKWebView to become first responder, EventHandler would
bail from setting the selection, assuming UIKit was going to do it. This
behavior was introduced in r233311.

However, since we are not first responder, UIKit does not change the
selection, since it considers the view to not be editable.

Fix this by letting WebCore set the selection in this case, as it used to.

  • WebProcess/WebCoreSupport/WebEditorClient.h:
  • WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm: (WebKit::WebEditorClient::shouldAllowSingleClickToChangeSelection const):
  • WebProcess/WebPage/WebPage.h: (WebKit::WebPage::isShowingInputViewForFocusedElement const): Copy the logic from EventHandler, with the added caveat (which fixes the aforementioned behavior) that we will allow EventHandler to change the selection if we don't have a focused node in the UIKit sense, because we know that the platform text interaction code will *not* change the selection if that is the case, so it's up to us.

Source/WebKitLegacy/mac:

  • WebCoreSupport/WebEditorClient.h:
  • WebCoreSupport/WebEditorClient.mm: (WebEditorClient::shouldAllowSingleClickToChangeSelection const): Copy the existing behavior from EventHandler. We do not fix the bug in WebKitLegacy for a multitude of reasons, primarily because we do not know of any user impact.

LayoutTests:

  • editing/selection/ios/change-selection-by-tapping-with-existing-selection-expected.txt: Added.
  • editing/selection/ios/change-selection-by-tapping-with-existing-selection.html: Added.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@248974 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-608-branch
Files:
2 added
11 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-608-branch/LayoutTests/ChangeLog

    r248982 r248988  
     12019-08-21  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r248974. rdar://problem/54579634
     4
     5    [Mail] Tapping top of message scrolls back to copied text instead of top of the message
     6    https://bugs.webkit.org/show_bug.cgi?id=200999
     7    <rdar://problem/54564878>
     8   
     9    Reviewed by Wenson Hsieh.
     10   
     11    Source/WebCore:
     12   
     13    Test: editing/selection/ios/change-selection-by-tapping-with-existing-selection.html
     14   
     15    * page/EditorClient.h:
     16    (WebCore::EditorClient::shouldAllowSingleClickToChangeSelection const):
     17    * page/EventHandler.cpp:
     18    (WebCore::EventHandler::handleMousePressEventSingleClick):
     19    Instead of encoding platform behaviors in EventHandler, defer to EditorClient.
     20   
     21    Source/WebKit:
     22   
     23    In the case where you have a WebCore selection but are not first responder,
     24    when you tap the WKWebView to become first responder, EventHandler would
     25    bail from setting the selection, assuming UIKit was going to do it. This
     26    behavior was introduced in r233311.
     27   
     28    However, since we are not first responder, UIKit does not change the
     29    selection, since it considers the view to not be editable.
     30   
     31    Fix this by letting WebCore set the selection in this case, as it used to.
     32   
     33    * WebProcess/WebCoreSupport/WebEditorClient.h:
     34    * WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm:
     35    (WebKit::WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     36    * WebProcess/WebPage/WebPage.h:
     37    (WebKit::WebPage::isShowingInputViewForFocusedElement const):
     38    Copy the logic from EventHandler, with the added caveat (which fixes the
     39    aforementioned behavior) that we will allow EventHandler to change the
     40    selection if we don't have a focused node in the UIKit sense, because
     41    we know that the platform text interaction code will *not* change the
     42    selection if that is the case, so it's up to us.
     43   
     44    Source/WebKitLegacy/mac:
     45   
     46    * WebCoreSupport/WebEditorClient.h:
     47    * WebCoreSupport/WebEditorClient.mm:
     48    (WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     49    Copy the existing behavior from EventHandler.
     50    We do not fix the bug in WebKitLegacy for a multitude of reasons, primarily
     51    because we do not know of any user impact.
     52   
     53    LayoutTests:
     54   
     55    * editing/selection/ios/change-selection-by-tapping-with-existing-selection-expected.txt: Added.
     56    * editing/selection/ios/change-selection-by-tapping-with-existing-selection.html: Added.
     57   
     58   
     59    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248974 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     60
     61    2019-08-21  Tim Horton  <timothy_horton@apple.com>
     62
     63            [Mail] Tapping top of message scrolls back to copied text instead of top of the message
     64            https://bugs.webkit.org/show_bug.cgi?id=200999
     65            <rdar://problem/54564878>
     66
     67            Reviewed by Wenson Hsieh.
     68
     69            * editing/selection/ios/change-selection-by-tapping-with-existing-selection-expected.txt: Added.
     70            * editing/selection/ios/change-selection-by-tapping-with-existing-selection.html: Added.
     71
    1722019-08-21  Ryan Haddad  <ryanhaddad@apple.com>
    273

  • branches/safari-608-branch/Source/WebCore/ChangeLog

    r248986 r248988  
     12019-08-21  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r248974. rdar://problem/54579634
     4
     5    [Mail] Tapping top of message scrolls back to copied text instead of top of the message
     6    https://bugs.webkit.org/show_bug.cgi?id=200999
     7    <rdar://problem/54564878>
     8   
     9    Reviewed by Wenson Hsieh.
     10   
     11    Source/WebCore:
     12   
     13    Test: editing/selection/ios/change-selection-by-tapping-with-existing-selection.html
     14   
     15    * page/EditorClient.h:
     16    (WebCore::EditorClient::shouldAllowSingleClickToChangeSelection const):
     17    * page/EventHandler.cpp:
     18    (WebCore::EventHandler::handleMousePressEventSingleClick):
     19    Instead of encoding platform behaviors in EventHandler, defer to EditorClient.
     20   
     21    Source/WebKit:
     22   
     23    In the case where you have a WebCore selection but are not first responder,
     24    when you tap the WKWebView to become first responder, EventHandler would
     25    bail from setting the selection, assuming UIKit was going to do it. This
     26    behavior was introduced in r233311.
     27   
     28    However, since we are not first responder, UIKit does not change the
     29    selection, since it considers the view to not be editable.
     30   
     31    Fix this by letting WebCore set the selection in this case, as it used to.
     32   
     33    * WebProcess/WebCoreSupport/WebEditorClient.h:
     34    * WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm:
     35    (WebKit::WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     36    * WebProcess/WebPage/WebPage.h:
     37    (WebKit::WebPage::isShowingInputViewForFocusedElement const):
     38    Copy the logic from EventHandler, with the added caveat (which fixes the
     39    aforementioned behavior) that we will allow EventHandler to change the
     40    selection if we don't have a focused node in the UIKit sense, because
     41    we know that the platform text interaction code will *not* change the
     42    selection if that is the case, so it's up to us.
     43   
     44    Source/WebKitLegacy/mac:
     45   
     46    * WebCoreSupport/WebEditorClient.h:
     47    * WebCoreSupport/WebEditorClient.mm:
     48    (WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     49    Copy the existing behavior from EventHandler.
     50    We do not fix the bug in WebKitLegacy for a multitude of reasons, primarily
     51    because we do not know of any user impact.
     52   
     53    LayoutTests:
     54   
     55    * editing/selection/ios/change-selection-by-tapping-with-existing-selection-expected.txt: Added.
     56    * editing/selection/ios/change-selection-by-tapping-with-existing-selection.html: Added.
     57   
     58   
     59    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248974 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     60
     61    2019-08-21  Tim Horton  <timothy_horton@apple.com>
     62
     63            [Mail] Tapping top of message scrolls back to copied text instead of top of the message
     64            https://bugs.webkit.org/show_bug.cgi?id=200999
     65            <rdar://problem/54564878>
     66
     67            Reviewed by Wenson Hsieh.
     68
     69            Test: editing/selection/ios/change-selection-by-tapping-with-existing-selection.html
     70
     71            * page/EditorClient.h:
     72            (WebCore::EditorClient::shouldAllowSingleClickToChangeSelection const):
     73            * page/EventHandler.cpp:
     74            (WebCore::EventHandler::handleMousePressEventSingleClick):
     75            Instead of encoding platform behaviors in EventHandler, defer to EditorClient.
     76
    1772019-08-21  Kocsen Chung  <kocsen_chung@apple.com>
    278

  • branches/safari-608-branch/Source/WebCore/page/EditorClient.h

    r247967 r248988  
    187187
    188188    virtual bool canShowFontPanel() const = 0;
     189
     190    virtual bool shouldAllowSingleClickToChangeSelection(Node&, const VisibleSelection&) const { return true; }
    189191};
    190192

  • branches/safari-608-branch/Source/WebCore/page/EventHandler.cpp

    r247606 r248988  
    699699    TextGranularity granularity = CharacterGranularity;
    700700
    701 #if PLATFORM(IOS_FAMILY)
    702     // The text selection assistant will handle selection in the case where we are already editing the node
    703     auto* editableRoot = newSelection.rootEditableElement();
    704     if (editableRoot && editableRoot == targetNode->rootEditableElement())
     701    if (!m_frame.editor().client()->shouldAllowSingleClickToChangeSelection(*targetNode, newSelection))
    705702        return true;
    706 #endif
    707703
    708704    if (extendSelection && newSelection.isCaretOrRange()) {

  • branches/safari-608-branch/Source/WebKit/ChangeLog

    r248987 r248988  
     12019-08-21  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r248974. rdar://problem/54579634
     4
     5    [Mail] Tapping top of message scrolls back to copied text instead of top of the message
     6    https://bugs.webkit.org/show_bug.cgi?id=200999
     7    <rdar://problem/54564878>
     8   
     9    Reviewed by Wenson Hsieh.
     10   
     11    Source/WebCore:
     12   
     13    Test: editing/selection/ios/change-selection-by-tapping-with-existing-selection.html
     14   
     15    * page/EditorClient.h:
     16    (WebCore::EditorClient::shouldAllowSingleClickToChangeSelection const):
     17    * page/EventHandler.cpp:
     18    (WebCore::EventHandler::handleMousePressEventSingleClick):
     19    Instead of encoding platform behaviors in EventHandler, defer to EditorClient.
     20   
     21    Source/WebKit:
     22   
     23    In the case where you have a WebCore selection but are not first responder,
     24    when you tap the WKWebView to become first responder, EventHandler would
     25    bail from setting the selection, assuming UIKit was going to do it. This
     26    behavior was introduced in r233311.
     27   
     28    However, since we are not first responder, UIKit does not change the
     29    selection, since it considers the view to not be editable.
     30   
     31    Fix this by letting WebCore set the selection in this case, as it used to.
     32   
     33    * WebProcess/WebCoreSupport/WebEditorClient.h:
     34    * WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm:
     35    (WebKit::WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     36    * WebProcess/WebPage/WebPage.h:
     37    (WebKit::WebPage::isShowingInputViewForFocusedElement const):
     38    Copy the logic from EventHandler, with the added caveat (which fixes the
     39    aforementioned behavior) that we will allow EventHandler to change the
     40    selection if we don't have a focused node in the UIKit sense, because
     41    we know that the platform text interaction code will *not* change the
     42    selection if that is the case, so it's up to us.
     43   
     44    Source/WebKitLegacy/mac:
     45   
     46    * WebCoreSupport/WebEditorClient.h:
     47    * WebCoreSupport/WebEditorClient.mm:
     48    (WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     49    Copy the existing behavior from EventHandler.
     50    We do not fix the bug in WebKitLegacy for a multitude of reasons, primarily
     51    because we do not know of any user impact.
     52   
     53    LayoutTests:
     54   
     55    * editing/selection/ios/change-selection-by-tapping-with-existing-selection-expected.txt: Added.
     56    * editing/selection/ios/change-selection-by-tapping-with-existing-selection.html: Added.
     57   
     58   
     59    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248974 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     60
     61    2019-08-21  Tim Horton  <timothy_horton@apple.com>
     62
     63            [Mail] Tapping top of message scrolls back to copied text instead of top of the message
     64            https://bugs.webkit.org/show_bug.cgi?id=200999
     65            <rdar://problem/54564878>
     66
     67            Reviewed by Wenson Hsieh.
     68
     69            In the case where you have a WebCore selection but are not first responder,
     70            when you tap the WKWebView to become first responder, EventHandler would
     71            bail from setting the selection, assuming UIKit was going to do it. This
     72            behavior was introduced in r233311.
     73
     74            However, since we are not first responder, UIKit does not change the
     75            selection, since it considers the view to not be editable.
     76
     77            Fix this by letting WebCore set the selection in this case, as it used to.
     78
     79            * WebProcess/WebCoreSupport/WebEditorClient.h:
     80            * WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm:
     81            (WebKit::WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     82            * WebProcess/WebPage/WebPage.h:
     83            (WebKit::WebPage::isShowingInputViewForFocusedElement const):
     84            Copy the logic from EventHandler, with the added caveat (which fixes the
     85            aforementioned behavior) that we will allow EventHandler to change the
     86            selection if we don't have a focused node in the UIKit sense, because
     87            we know that the platform text interaction code will *not* change the
     88            selection if that is the case, so it's up to us.
     89
    1902019-08-21  Kocsen Chung  <kocsen_chung@apple.com>
    291

  • branches/safari-608-branch/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.h

    r247967 r248988  
    181181    bool performsTwoStepPaste(WebCore::DocumentFragment*) final;
    182182    void updateStringForFind(const String&) final;
     183    bool shouldAllowSingleClickToChangeSelection(WebCore::Node&, const WebCore::VisibleSelection&) const final;
    183184#endif
    184185

  • branches/safari-608-branch/Source/WebKit/WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm

    r244982 r248988  
    103103}
    104104
     105bool WebEditorClient::shouldAllowSingleClickToChangeSelection(WebCore::Node& targetNode, const WebCore::VisibleSelection& newSelection) const
     106{
     107    // The text selection assistant will handle selection in the case where we are already editing the node
     108    auto* editableRoot = newSelection.rootEditableElement();
     109    return !editableRoot || editableRoot != targetNode.rootEditableElement() || !m_page->isShowingInputViewForFocusedElement();
     110}
     111
    105112} // namespace WebKit
    106113

  • branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.h

    r248568 r248988  
    676676    void setFocusedElementSelectedIndex(uint32_t index, bool allowMultipleSelection);
    677677    void setIsShowingInputViewForFocusedElement(bool);
     678    bool isShowingInputViewForFocusedElement() const { return m_isShowingInputViewForFocusedElement; }
    678679    void updateSelectionAppearance();
    679680    void getSelectionContext(CallbackID);

  • branches/safari-608-branch/Source/WebKitLegacy/mac/ChangeLog

    r248745 r248988  
     12019-08-21  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r248974. rdar://problem/54579634
     4
     5    [Mail] Tapping top of message scrolls back to copied text instead of top of the message
     6    https://bugs.webkit.org/show_bug.cgi?id=200999
     7    <rdar://problem/54564878>
     8   
     9    Reviewed by Wenson Hsieh.
     10   
     11    Source/WebCore:
     12   
     13    Test: editing/selection/ios/change-selection-by-tapping-with-existing-selection.html
     14   
     15    * page/EditorClient.h:
     16    (WebCore::EditorClient::shouldAllowSingleClickToChangeSelection const):
     17    * page/EventHandler.cpp:
     18    (WebCore::EventHandler::handleMousePressEventSingleClick):
     19    Instead of encoding platform behaviors in EventHandler, defer to EditorClient.
     20   
     21    Source/WebKit:
     22   
     23    In the case where you have a WebCore selection but are not first responder,
     24    when you tap the WKWebView to become first responder, EventHandler would
     25    bail from setting the selection, assuming UIKit was going to do it. This
     26    behavior was introduced in r233311.
     27   
     28    However, since we are not first responder, UIKit does not change the
     29    selection, since it considers the view to not be editable.
     30   
     31    Fix this by letting WebCore set the selection in this case, as it used to.
     32   
     33    * WebProcess/WebCoreSupport/WebEditorClient.h:
     34    * WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm:
     35    (WebKit::WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     36    * WebProcess/WebPage/WebPage.h:
     37    (WebKit::WebPage::isShowingInputViewForFocusedElement const):
     38    Copy the logic from EventHandler, with the added caveat (which fixes the
     39    aforementioned behavior) that we will allow EventHandler to change the
     40    selection if we don't have a focused node in the UIKit sense, because
     41    we know that the platform text interaction code will *not* change the
     42    selection if that is the case, so it's up to us.
     43   
     44    Source/WebKitLegacy/mac:
     45   
     46    * WebCoreSupport/WebEditorClient.h:
     47    * WebCoreSupport/WebEditorClient.mm:
     48    (WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     49    Copy the existing behavior from EventHandler.
     50    We do not fix the bug in WebKitLegacy for a multitude of reasons, primarily
     51    because we do not know of any user impact.
     52   
     53    LayoutTests:
     54   
     55    * editing/selection/ios/change-selection-by-tapping-with-existing-selection-expected.txt: Added.
     56    * editing/selection/ios/change-selection-by-tapping-with-existing-selection.html: Added.
     57   
     58   
     59    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248974 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     60
     61    2019-08-21  Tim Horton  <timothy_horton@apple.com>
     62
     63            [Mail] Tapping top of message scrolls back to copied text instead of top of the message
     64            https://bugs.webkit.org/show_bug.cgi?id=200999
     65            <rdar://problem/54564878>
     66
     67            Reviewed by Wenson Hsieh.
     68
     69            * WebCoreSupport/WebEditorClient.h:
     70            * WebCoreSupport/WebEditorClient.mm:
     71            (WebEditorClient::shouldAllowSingleClickToChangeSelection const):
     72            Copy the existing behavior from EventHandler.
     73            We do not fix the bug in WebKitLegacy for a multitude of reasons, primarily
     74            because we do not know of any user impact.
     75
    1762019-08-15  Alan Coon  <alancoon@apple.com>
    277

  • branches/safari-608-branch/Source/WebKitLegacy/mac/WebCoreSupport/WebEditorClient.h

    r247967 r248988  
    176176    void registerUndoOrRedoStep(WebCore::UndoStep&, bool isRedo);
    177177
     178#if PLATFORM(IOS_FAMILY)
     179    bool shouldAllowSingleClickToChangeSelection(WebCore::Node& targetNode, const WebCore::VisibleSelection& newSelection) const;
     180#endif
     181
    178182    bool canShowFontPanel() const final
    179183    {

  • branches/safari-608-branch/Source/WebKitLegacy/mac/WebCoreSupport/WebEditorClient.mm

    r244932 r248988  
    12591259#endif
    12601260}
     1261
     1262#if PLATFORM(IOS_FAMILY)
     1263bool WebEditorClient::shouldAllowSingleClickToChangeSelection(WebCore::Node& targetNode, const WebCore::VisibleSelection& newSelection) const
     1264{
     1265    // The text selection assistant will handle selection in the case where we are already editing the node
     1266    auto* editableRoot = newSelection.rootEditableElement();
     1267    return !editableRoot || editableRoot != targetNode.rootEditableElement();
     1268}
     1269#endif
Note: See TracChangeset for help on using the changeset viewer.