[iOS WK2] Implement -[WKContentView hasText] for compatibility with the UITextInput...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Sep 2017 18:33:32 +0000 (18:33 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Sep 2017 18:33:32 +0000 (18:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177662
<rdar://problem/33410373>

Reviewed by Tim Horton.

Source/WebCore:

Adds a new TextIterator helper function to determine whether a Range has any plain text.

Tests: EditorStateTests.ContentViewHasTextInContentEditableElement
       EditorStateTests.ContentViewHasTextInTextarea

* editing/TextIterator.cpp:
(WebCore::hasAnyPlainText):

Add a new helper to determine whether a Range contains any plain text. While inspired by and very similar to the
plainText() helper function, this variant does not create a new string buffer when invoked, and is therefore
more efficient for the purposes of determining whether there is any plain text at all.

* editing/TextIterator.h:

Source/WebKit:

Implements -[WKContentView hasText] by propagating a flag through post-layout editor state.

* Shared/EditorState.cpp:
(WebKit::EditorState::PostLayoutData::encode const):
(WebKit::EditorState::PostLayoutData::decode):
* Shared/EditorState.h:

Add a new flag to EditorState indicating whether or not the current editable root containing the selection has
any plain text. Add IPC support for this new flag.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView hasText]):
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::computeEditableRootHasContentAndPlainText):

Add a new helper to compute whether or not the editable root has any content, and any plain text. This
is used as the last cached value for -hasText on WKContentView that we will deliver to UIKit. Some important
things to note here:
- If post layout data already indicates that we have selected some plain text, or that there is a plain text
  character near the selection, just set the flags to true and bail, since the editable root necessarily has
  content that is plain text.
- If hasContent is false, don't even bother computing hasPlainText, because it must also be false.
- Otherwise, use hasAnyPlainText to compute the value of hasPlainText, which is a faster variant of plainText.
These optimizations help us avoid doing extra work at all when running Speedometer, apart from checking the
values of a few PostLayoutData flags. This also fixes the value of hasContent, which was previously always false
if we had a range selection rather than a caret selection even when the editable root has content, because the
logic to compute the value of hasContent only existed in the branch where we have a caret selection.

(WebKit::WebPage::platformEditorState const):

Tools:

Add EditorState API tests to check that the value of -[WKContentView hasText] is correct when editing both plain
and rich text areas.

* TestWebKitAPI/EditingTestHarness.h:
* TestWebKitAPI/EditingTestHarness.mm:
(-[EditingTestHarness insertParagraph]):
(-[EditingTestHarness insertText:]):
(-[EditingTestHarness insertHTML:]):
(-[EditingTestHarness selectAll]):
(-[EditingTestHarness deleteBackwards]):
* TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm:

Add versions of EditingTestHarness helpers that don't require us to expect any editor state after executing the
edit command.

(TestWebKitAPI::checkContentViewHasTextWithFailureDescription):
(TestWebKitAPI::TEST):
* TestWebKitAPI/cocoa/TestWKWebView.h:
* TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView textInputContentView]):

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

14 files changed:
Source/WebCore/ChangeLog
Source/WebCore/editing/TextIterator.cpp
Source/WebCore/editing/TextIterator.h
Source/WebKit/ChangeLog
Source/WebKit/Shared/EditorState.cpp
Source/WebKit/Shared/EditorState.h
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Tools/ChangeLog
Tools/TestWebKitAPI/EditingTestHarness.h
Tools/TestWebKitAPI/EditingTestHarness.mm
Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm
Tools/TestWebKitAPI/cocoa/TestWKWebView.h
Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

index 90b0bbc2c7f4c062d52f64bf5da6111684af275b..a8d7f2631f8d1608a0e0154f76a9a20ecf340a0c 100644 (file)
@@ -1,3 +1,25 @@
+2017-09-29  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS WK2] Implement -[WKContentView hasText] for compatibility with the UITextInput protocol
+        https://bugs.webkit.org/show_bug.cgi?id=177662
+        <rdar://problem/33410373>
+
+        Reviewed by Tim Horton.
+
+        Adds a new TextIterator helper function to determine whether a Range has any plain text.
+
+        Tests: EditorStateTests.ContentViewHasTextInContentEditableElement
+               EditorStateTests.ContentViewHasTextInTextarea
+
+        * editing/TextIterator.cpp:
+        (WebCore::hasAnyPlainText):
+
+        Add a new helper to determine whether a Range contains any plain text. While inspired by and very similar to the
+        plainText() helper function, this variant does not create a new string buffer when invoked, and is therefore
+        more efficient for the purposes of determining whether there is any plain text at all.
+
+        * editing/TextIterator.h:
+
 2017-09-29  Zalan Bujtas  <zalan@apple.com>
 
         Add WeakPtr support to RenderObject.
index 552da652ba372b8676507d1c2739beb2d8b98a0e..d528305a238ffa32723c924b0ea34821d0bcd98c 100644 (file)
@@ -2634,6 +2634,15 @@ bool TextIterator::getLocationAndLengthFromRange(Node* scope, const Range* range
 
 // --------
 
+bool hasAnyPlainText(const Range& range, TextIteratorBehavior behavior)
+{
+    for (TextIterator iterator { &range, behavior }; !iterator.atEnd(); iterator.advance()) {
+        if (!iterator.text().isEmpty())
+            return true;
+    }
+    return false;
+}
+
 String plainText(const Range* r, TextIteratorBehavior defaultBehavior, bool isDisplayString)
 {
     // The initial buffer size can be critical for performance: https://bugs.webkit.org/show_bug.cgi?id=81192
index 4064612d59fe2d1f046ce52aad5ca9575b45af85..23c0b5ec75b37063afc21cb83dead3bb5a780b0d 100644 (file)
@@ -46,6 +46,7 @@ WEBCORE_EXPORT String plainText(const Range*, TextIteratorBehavior = TextIterato
 WEBCORE_EXPORT String plainTextReplacingNoBreakSpace(const Range*, TextIteratorBehavior = TextIteratorDefaultBehavior, bool isDisplayString = false);
 Ref<Range> findPlainText(const Range&, const String&, FindOptions);
 WEBCORE_EXPORT Ref<Range> findClosestPlainText(const Range&, const String&, FindOptions, unsigned);
+WEBCORE_EXPORT bool hasAnyPlainText(const Range&, TextIteratorBehavior = TextIteratorDefaultBehavior);
 
 // FIXME: Move this somewhere else in the editing directory. It doesn't belong here.
 bool isRendererReplacedElement(RenderObject*);
index 00e04c66b0ed7961b631604219ba8a215c01ff09..6894e935f5725594a9bd2fac45ddce07d5d372f4 100644 (file)
@@ -1,3 +1,41 @@
+2017-09-29  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS WK2] Implement -[WKContentView hasText] for compatibility with the UITextInput protocol
+        https://bugs.webkit.org/show_bug.cgi?id=177662
+        <rdar://problem/33410373>
+
+        Reviewed by Tim Horton.
+
+        Implements -[WKContentView hasText] by propagating a flag through post-layout editor state.
+
+        * Shared/EditorState.cpp:
+        (WebKit::EditorState::PostLayoutData::encode const):
+        (WebKit::EditorState::PostLayoutData::decode):
+        * Shared/EditorState.h:
+
+        Add a new flag to EditorState indicating whether or not the current editable root containing the selection has
+        any plain text. Add IPC support for this new flag.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView hasText]):
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::computeEditableRootHasContentAndPlainText):
+
+        Add a new helper to compute whether or not the editable root has any content, and any plain text. This
+        is used as the last cached value for -hasText on WKContentView that we will deliver to UIKit. Some important
+        things to note here:
+        - If post layout data already indicates that we have selected some plain text, or that there is a plain text
+          character near the selection, just set the flags to true and bail, since the editable root necessarily has
+          content that is plain text.
+        - If hasContent is false, don't even bother computing hasPlainText, because it must also be false.
+        - Otherwise, use hasAnyPlainText to compute the value of hasPlainText, which is a faster variant of plainText.
+        These optimizations help us avoid doing extra work at all when running Speedometer, apart from checking the
+        values of a few PostLayoutData flags. This also fixes the value of hasContent, which was previously always false
+        if we had a range selection rather than a caret selection even when the editable root has content, because the
+        logic to compute the value of hasContent only existed in the branch where we have a caret selection.
+
+        (WebKit::WebPage::platformEditorState const):
+
 2017-09-28  Timothy Horton  <timothy_horton@apple.com>
 
         Fix the macOS CMake build
index 6f9cf48bebf0bb291d36654b98f3f8d0a5146391..5afac59abedbaad0173146f7c202137a4faae318 100644 (file)
@@ -127,6 +127,7 @@ void EditorState::PostLayoutData::encode(IPC::Encoder& encoder) const
     encoder << hasContent;
     encoder << isStableStateUpdate;
     encoder << insideFixedPosition;
+    encoder << hasPlainText;
 #endif
 #if PLATFORM(MAC)
     encoder << candidateRequestStartPosition;
@@ -176,6 +177,8 @@ bool EditorState::PostLayoutData::decode(IPC::Decoder& decoder, PostLayoutData&
         return false;
     if (!decoder.decode(result.insideFixedPosition))
         return false;
+    if (!decoder.decode(result.hasPlainText))
+        return false;
 #endif
 #if PLATFORM(MAC)
     if (!decoder.decode(result.candidateRequestStartPosition))
index 3345dc4c4faf149b3eac409779aacbd592d0d71d..b705a6e98f835f3b0f205ad04ed31b3328013685 100644 (file)
@@ -101,6 +101,7 @@ struct EditorState {
         bool hasContent { false };
         bool isStableStateUpdate { false };
         bool insideFixedPosition { false };
+        bool hasPlainText { false };
 #endif
 #if PLATFORM(MAC)
         uint64_t candidateRequestStartPosition { 0 };
index 9291764fb6bfb17bbcc9153f5bc635cdcb5af785..fa9a8128adc0a01c07e8a94e2be88c2bc0db4e0a 100644 (file)
@@ -3182,7 +3182,8 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
 
 - (BOOL)hasText
 {
-    return YES;
+    auto& editorState = _page->editorState();
+    return !editorState.isMissingPostLayoutData && editorState.postLayoutData().hasPlainText;
 }
 
 // end of UITextInput protocol implementation
index 88d21dfc724bbf6a5536032ab26069998f3ef579..0b22c234104b4fa5b0855d73909f3d0bf85ae28d 100644 (file)
@@ -145,6 +145,29 @@ void WebPage::platformPreferencesDidChange(const WebPreferencesStore&)
     notImplemented();
 }
 
+static void computeEditableRootHasContentAndPlainText(const VisibleSelection& selection, EditorState::PostLayoutData& data)
+{
+    data.hasContent = false;
+    data.hasPlainText = false;
+    if (!selection.isContentEditable())
+        return;
+
+    if (data.selectedTextLength || data.characterAfterSelection || data.characterBeforeSelection || data.twoCharacterBeforeSelection) {
+        // If any of these variables have been previously set, the editable root must have plain text content, so we can bail from the remainder of the check.
+        data.hasContent = true;
+        data.hasPlainText = true;
+        return;
+    }
+
+    auto* root = selection.rootEditableElement();
+    if (!root)
+        return;
+
+    auto startInEditableRoot = firstPositionInNode(root);
+    data.hasContent = root->hasChildNodes() && !isEndOfEditableOrNonEditableContent(startInEditableRoot);
+    data.hasPlainText = data.hasContent && hasAnyPlainText(Range::create(root->document(), VisiblePosition { startInEditableRoot }, VisiblePosition { lastPositionInNode(root) }));
+}
+
 void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePostLayoutDataHint shouldIncludePostLayoutData) const
 {
     if (frame.editor().hasComposition()) {
@@ -185,11 +208,8 @@ void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePost
         // FIXME: The following check should take into account writing direction.
         postLayoutData.isReplaceAllowed = result.isContentEditable && atBoundaryOfGranularity(selection.start(), WordGranularity, DirectionForward);
         postLayoutData.wordAtSelection = plainTextReplacingNoBreakSpace(wordRangeFromPosition(selection.start()).get());
-        if (selection.isContentEditable()) {
+        if (selection.isContentEditable())
             charactersAroundPosition(selection.start(), postLayoutData.characterAfterSelection, postLayoutData.characterBeforeSelection, postLayoutData.twoCharacterBeforeSelection);
-            Node* root = selection.rootEditableElement();
-            postLayoutData.hasContent = root && root->hasChildNodes() && !isEndOfEditableOrNonEditableContent(firstPositionInNode(root));
-        }
     } else if (selection.isRange()) {
         postLayoutData.caretRectAtStart = view->contentsToRootView(VisiblePosition(selection.start()).absoluteCaretBounds(&startNodeIsInsideFixedPosition));
         postLayoutData.caretRectAtEnd = view->contentsToRootView(VisiblePosition(selection.end()).absoluteCaretBounds(&endNodeIsInsideFixedPosition));
@@ -211,6 +231,7 @@ void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePost
     if (!selection.isNone()) {
         if (m_assistedNode && m_assistedNode->renderer())
             postLayoutData.selectionClipRect = view->contentsToRootView(m_assistedNode->renderer()->absoluteBoundingBoxRect());
+        computeEditableRootHasContentAndPlainText(selection, postLayoutData);
     }
 }
 
index ef02b59ca52c4bb391e1471e669d048bb6c6e633..4322f7470c6210bf76030106db952e57f9981743 100644 (file)
@@ -1,3 +1,32 @@
+2017-09-29  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS WK2] Implement -[WKContentView hasText] for compatibility with the UITextInput protocol
+        https://bugs.webkit.org/show_bug.cgi?id=177662
+        <rdar://problem/33410373>
+
+        Reviewed by Tim Horton.
+
+        Add EditorState API tests to check that the value of -[WKContentView hasText] is correct when editing both plain
+        and rich text areas.
+
+        * TestWebKitAPI/EditingTestHarness.h:
+        * TestWebKitAPI/EditingTestHarness.mm:
+        (-[EditingTestHarness insertParagraph]):
+        (-[EditingTestHarness insertText:]):
+        (-[EditingTestHarness insertHTML:]):
+        (-[EditingTestHarness selectAll]):
+        (-[EditingTestHarness deleteBackwards]):
+        * TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm:
+
+        Add versions of EditingTestHarness helpers that don't require us to expect any editor state after executing the
+        edit command.
+
+        (TestWebKitAPI::checkContentViewHasTextWithFailureDescription):
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/cocoa/TestWKWebView.h:
+        * TestWebKitAPI/cocoa/TestWKWebView.mm:
+        (-[TestWKWebView textInputContentView]):
+
 2017-09-29  Charles Turner  <cturner@igalia.com>
 
         Update my status.
index f05b6b2cda8504d8ebd42174f05bd727691bd149..33e26c3624721400e90400d8259ea77e1f48455d 100644 (file)
 @property (nonatomic, readonly) NSDictionary *latestEditorState;
 @property (nonatomic, readonly) NSArray<NSDictionary *> *editorStateHistory;
 
+- (void)insertParagraph;
+- (void)insertText:(NSString *)text;
+- (void)insertHTML:(NSString *)html;
+- (void)selectAll;
+- (void)deleteBackwards;
 - (void)insertParagraphAndExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries;
 - (void)insertText:(NSString *)text andExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries;
 - (void)insertHTML:(NSString *)html andExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries;
index ac6adc15391eff771a6ea04ff2bfc81559a0a865..92b1b9eeae85beab9ed859f72bfdce2ce609304c 100644 (file)
     return _editorStateHistory.get();
 }
 
+- (void)insertParagraph
+{
+    [self insertParagraphAndExpectEditorStateWith:nil];
+}
+
+- (void)insertText:(NSString *)text
+{
+    [self insertText:text andExpectEditorStateWith:nil];
+}
+
+- (void)insertHTML:(NSString *)html
+{
+    [self insertHTML:html andExpectEditorStateWith:nil];
+}
+
+- (void)selectAll
+{
+    [self selectAllAndExpectEditorStateWith:nil];
+}
+
+- (void)deleteBackwards
+{
+    [self deleteBackwardAndExpectEditorStateWith:nil];
+}
+
 - (void)insertText:(NSString *)text andExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries
 {
     [self _execCommand:@"InsertText" argument:text expectEntries:entries];
index faa552edfab8db00cafbf60278e01c548d2776f8..8efbb90e82a2565877497700767af4daa6667292 100644 (file)
 #import "TestWKWebView.h"
 #import <WebKit/WKWebViewPrivate.h>
 
+#if PLATFORM(IOS)
+#import <UIKit/UIKit.h>
+#endif
+
 namespace TestWebKitAPI {
 
 static RetainPtr<EditingTestHarness> setUpEditorStateTestHarness()
@@ -229,6 +233,73 @@ TEST(EditorStateTests, TypingAttributeLinkColor)
     EXPECT_WK_STREQ("https://www.apple.com/", [[testHarness webView] stringByEvaluatingJavaScript:@"document.querySelector('a').href"]);
 }
 
+#if PLATFORM(IOS)
+
+static void checkContentViewHasTextWithFailureDescription(TestWKWebView *webView, BOOL expectedToHaveText, NSString *description)
+{
+    BOOL hasText = webView.textInputContentView.hasText;
+    if (expectedToHaveText)
+        EXPECT_TRUE(hasText);
+    else
+        EXPECT_FALSE(hasText);
+
+    if (expectedToHaveText != hasText)
+        NSLog(@"Expected -[%@ hasText] to be %@, but observed: %@ (%@)", [webView.textInputContentView class], expectedToHaveText ? @"YES" : @"NO", hasText ? @"YES" : @"NO", description);
+}
+
+TEST(EditorStateTests, ContentViewHasTextInContentEditableElement)
+{
+    auto testHarness = setUpEditorStateTestHarness();
+    TestWKWebView *webView = [testHarness webView];
+
+    checkContentViewHasTextWithFailureDescription(webView, NO, @"before inserting any content");
+    [testHarness insertHTML:@"<img src='icon.png'></img>"];
+    checkContentViewHasTextWithFailureDescription(webView, NO, @"after inserting an image element");
+    [testHarness insertText:@"A"];
+    checkContentViewHasTextWithFailureDescription(webView, YES, @"after inserting text");
+    [testHarness selectAll];
+    checkContentViewHasTextWithFailureDescription(webView, YES, @"after selecting everything");
+    [testHarness deleteBackwards];
+    checkContentViewHasTextWithFailureDescription(webView, NO, @"after deleting everything");
+    [testHarness insertParagraph];
+    checkContentViewHasTextWithFailureDescription(webView, YES, @"after inserting a newline");
+    [testHarness deleteBackwards];
+    checkContentViewHasTextWithFailureDescription(webView, NO, @"after deleting the newline");
+    [testHarness insertText:@"B"];
+    checkContentViewHasTextWithFailureDescription(webView, YES, @"after inserting text again");
+    [webView stringByEvaluatingJavaScript:@"document.body.blur()"];
+    [webView waitForNextPresentationUpdate];
+    checkContentViewHasTextWithFailureDescription(webView, NO, @"after losing focus");
+}
+
+TEST(EditorStateTests, ContentViewHasTextInTextarea)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+    auto testHarness = adoptNS([[EditingTestHarness alloc] initWithWebView:webView.get()]);
+    [webView synchronouslyLoadHTMLString:@"<textarea id='textarea'></textarea>"];
+    [webView stringByEvaluatingJavaScript:@"textarea.focus()"];
+    [webView waitForNextPresentationUpdate];
+
+    checkContentViewHasTextWithFailureDescription(webView.get(), NO, @"before inserting any content");
+    [testHarness insertText:@"A"];
+    checkContentViewHasTextWithFailureDescription(webView.get(), YES, @"after inserting text");
+    [testHarness selectAll];
+    checkContentViewHasTextWithFailureDescription(webView.get(), YES, @"after selecting everything");
+    [testHarness deleteBackwards];
+    checkContentViewHasTextWithFailureDescription(webView.get(), NO, @"after deleting everything");
+    [testHarness insertParagraph];
+    checkContentViewHasTextWithFailureDescription(webView.get(), YES, @"after inserting a newline");
+    [testHarness deleteBackwards];
+    checkContentViewHasTextWithFailureDescription(webView.get(), NO, @"after deleting the newline");
+    [testHarness insertText:@"B"];
+    checkContentViewHasTextWithFailureDescription(webView.get(), YES, @"after inserting text again");
+    [webView stringByEvaluatingJavaScript:@"textarea.blur()"];
+    [webView waitForNextPresentationUpdate];
+    checkContentViewHasTextWithFailureDescription(webView.get(), NO, @"after losing focus");
+}
+
+#endif
+
 } // namespace TestWebKitAPI
 
 #endif // WK_API_ENABLED
index d6f6578e69ec6b17874209d85c0479f293d16d93..e552d96abfa7a2a42cabdf38255c3ca0515877e9 100644 (file)
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#if WK_API_ENABLED
+
 #import <WebKit/WebKit.h>
 #import <wtf/RetainPtr.h>
 
-#if WK_API_ENABLED
-
 @class _WKProcessPoolConfiguration;
 
 #if PLATFORM(IOS)
@@ -53,6 +53,7 @@
 
 #if PLATFORM(IOS)
 @interface TestWKWebView (IOSOnly)
+@property (nonatomic, readonly) UIView <UITextInput> *textInputContentView;
 @property (nonatomic, readonly) RetainPtr<NSArray> selectionRectsAfterPresentationUpdate;
 - (_WKActivatedElementInfo *)activatedElementAtPosition:(CGPoint)position;
 @end
index dc1a45d4063c9e6e5cda97def3f342e2041ece70..ef417fb97ad4ac98ce4079448193155990d07961 100644 (file)
@@ -45,6 +45,7 @@
 #endif
 
 #if PLATFORM(IOS)
+#import <UIKit/UIKit.h>
 #import <wtf/SoftLinking.h>
 SOFT_LINK_FRAMEWORK(UIKit)
 SOFT_LINK_CLASS(UIKit, UIWindow)
@@ -292,6 +293,11 @@ NSEventMask __simulated_forceClickAssociatedEventsMask(id self, SEL _cmd)
 
 @implementation TestWKWebView (IOSOnly)
 
+- (UIView <UITextInput> *)textInputContentView
+{
+    return (UIView <UITextInput> *)[self valueForKey:@"_currentContentView"];
+}
+
 - (RetainPtr<NSArray>)selectionRectsAfterPresentationUpdate
 {
     RetainPtr<TestWKWebView> retainedSelf = self;