Skip to content

Commit 000af40

Browse files
[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. 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: http://svn.webkit.org/repository/webkit/trunk@222654 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent fe2127a commit 000af40

File tree

14 files changed

+240
-7
lines changed

14 files changed

+240
-7
lines changed

Source/WebCore/ChangeLog

+22
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,25 @@
1+
2017-09-29 Wenson Hsieh <[email protected]>
2+
3+
[iOS WK2] Implement -[WKContentView hasText] for compatibility with the UITextInput protocol
4+
https://bugs.webkit.org/show_bug.cgi?id=177662
5+
<rdar://problem/33410373>
6+
7+
Reviewed by Tim Horton.
8+
9+
Adds a new TextIterator helper function to determine whether a Range has any plain text.
10+
11+
Tests: EditorStateTests.ContentViewHasTextInContentEditableElement
12+
EditorStateTests.ContentViewHasTextInTextarea
13+
14+
* editing/TextIterator.cpp:
15+
(WebCore::hasAnyPlainText):
16+
17+
Add a new helper to determine whether a Range contains any plain text. While inspired by and very similar to the
18+
plainText() helper function, this variant does not create a new string buffer when invoked, and is therefore
19+
more efficient for the purposes of determining whether there is any plain text at all.
20+
21+
* editing/TextIterator.h:
22+
123
2017-09-29 Zalan Bujtas <[email protected]>
224

325
Add WeakPtr support to RenderObject.

Source/WebCore/editing/TextIterator.cpp

+9
Original file line numberDiff line numberDiff line change
@@ -2634,6 +2634,15 @@ bool TextIterator::getLocationAndLengthFromRange(Node* scope, const Range* range
26342634

26352635
// --------
26362636

2637+
bool hasAnyPlainText(const Range& range, TextIteratorBehavior behavior)
2638+
{
2639+
for (TextIterator iterator { &range, behavior }; !iterator.atEnd(); iterator.advance()) {
2640+
if (!iterator.text().isEmpty())
2641+
return true;
2642+
}
2643+
return false;
2644+
}
2645+
26372646
String plainText(const Range* r, TextIteratorBehavior defaultBehavior, bool isDisplayString)
26382647
{
26392648
// The initial buffer size can be critical for performance: https://bugs.webkit.org/show_bug.cgi?id=81192

Source/WebCore/editing/TextIterator.h

+1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ WEBCORE_EXPORT String plainText(const Range*, TextIteratorBehavior = TextIterato
4646
WEBCORE_EXPORT String plainTextReplacingNoBreakSpace(const Range*, TextIteratorBehavior = TextIteratorDefaultBehavior, bool isDisplayString = false);
4747
Ref<Range> findPlainText(const Range&, const String&, FindOptions);
4848
WEBCORE_EXPORT Ref<Range> findClosestPlainText(const Range&, const String&, FindOptions, unsigned);
49+
WEBCORE_EXPORT bool hasAnyPlainText(const Range&, TextIteratorBehavior = TextIteratorDefaultBehavior);
4950

5051
// FIXME: Move this somewhere else in the editing directory. It doesn't belong here.
5152
bool isRendererReplacedElement(RenderObject*);

Source/WebKit/ChangeLog

+38
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,41 @@
1+
2017-09-29 Wenson Hsieh <[email protected]>
2+
3+
[iOS WK2] Implement -[WKContentView hasText] for compatibility with the UITextInput protocol
4+
https://bugs.webkit.org/show_bug.cgi?id=177662
5+
<rdar://problem/33410373>
6+
7+
Reviewed by Tim Horton.
8+
9+
Implements -[WKContentView hasText] by propagating a flag through post-layout editor state.
10+
11+
* Shared/EditorState.cpp:
12+
(WebKit::EditorState::PostLayoutData::encode const):
13+
(WebKit::EditorState::PostLayoutData::decode):
14+
* Shared/EditorState.h:
15+
16+
Add a new flag to EditorState indicating whether or not the current editable root containing the selection has
17+
any plain text. Add IPC support for this new flag.
18+
19+
* UIProcess/ios/WKContentViewInteraction.mm:
20+
(-[WKContentView hasText]):
21+
* WebProcess/WebPage/ios/WebPageIOS.mm:
22+
(WebKit::computeEditableRootHasContentAndPlainText):
23+
24+
Add a new helper to compute whether or not the editable root has any content, and any plain text. This
25+
is used as the last cached value for -hasText on WKContentView that we will deliver to UIKit. Some important
26+
things to note here:
27+
- If post layout data already indicates that we have selected some plain text, or that there is a plain text
28+
character near the selection, just set the flags to true and bail, since the editable root necessarily has
29+
content that is plain text.
30+
- If hasContent is false, don't even bother computing hasPlainText, because it must also be false.
31+
- Otherwise, use hasAnyPlainText to compute the value of hasPlainText, which is a faster variant of plainText.
32+
These optimizations help us avoid doing extra work at all when running Speedometer, apart from checking the
33+
values of a few PostLayoutData flags. This also fixes the value of hasContent, which was previously always false
34+
if we had a range selection rather than a caret selection even when the editable root has content, because the
35+
logic to compute the value of hasContent only existed in the branch where we have a caret selection.
36+
37+
(WebKit::WebPage::platformEditorState const):
38+
139
2017-09-28 Timothy Horton <[email protected]>
240

341
Fix the macOS CMake build

Source/WebKit/Shared/EditorState.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ void EditorState::PostLayoutData::encode(IPC::Encoder& encoder) const
127127
encoder << hasContent;
128128
encoder << isStableStateUpdate;
129129
encoder << insideFixedPosition;
130+
encoder << hasPlainText;
130131
#endif
131132
#if PLATFORM(MAC)
132133
encoder << candidateRequestStartPosition;
@@ -176,6 +177,8 @@ bool EditorState::PostLayoutData::decode(IPC::Decoder& decoder, PostLayoutData&
176177
return false;
177178
if (!decoder.decode(result.insideFixedPosition))
178179
return false;
180+
if (!decoder.decode(result.hasPlainText))
181+
return false;
179182
#endif
180183
#if PLATFORM(MAC)
181184
if (!decoder.decode(result.candidateRequestStartPosition))

Source/WebKit/Shared/EditorState.h

+1
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ struct EditorState {
101101
bool hasContent { false };
102102
bool isStableStateUpdate { false };
103103
bool insideFixedPosition { false };
104+
bool hasPlainText { false };
104105
#endif
105106
#if PLATFORM(MAC)
106107
uint64_t candidateRequestStartPosition { 0 };

Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

+2-1
Original file line numberDiff line numberDiff line change
@@ -3182,7 +3182,8 @@ - (void)insertText:(NSString *)aStringValue
31823182

31833183
- (BOOL)hasText
31843184
{
3185-
return YES;
3185+
auto& editorState = _page->editorState();
3186+
return !editorState.isMissingPostLayoutData && editorState.postLayoutData().hasPlainText;
31863187
}
31873188

31883189
// end of UITextInput protocol implementation

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

+25-4
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,29 @@
145145
notImplemented();
146146
}
147147

148+
static void computeEditableRootHasContentAndPlainText(const VisibleSelection& selection, EditorState::PostLayoutData& data)
149+
{
150+
data.hasContent = false;
151+
data.hasPlainText = false;
152+
if (!selection.isContentEditable())
153+
return;
154+
155+
if (data.selectedTextLength || data.characterAfterSelection || data.characterBeforeSelection || data.twoCharacterBeforeSelection) {
156+
// 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.
157+
data.hasContent = true;
158+
data.hasPlainText = true;
159+
return;
160+
}
161+
162+
auto* root = selection.rootEditableElement();
163+
if (!root)
164+
return;
165+
166+
auto startInEditableRoot = firstPositionInNode(root);
167+
data.hasContent = root->hasChildNodes() && !isEndOfEditableOrNonEditableContent(startInEditableRoot);
168+
data.hasPlainText = data.hasContent && hasAnyPlainText(Range::create(root->document(), VisiblePosition { startInEditableRoot }, VisiblePosition { lastPositionInNode(root) }));
169+
}
170+
148171
void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePostLayoutDataHint shouldIncludePostLayoutData) const
149172
{
150173
if (frame.editor().hasComposition()) {
@@ -185,11 +208,8 @@
185208
// FIXME: The following check should take into account writing direction.
186209
postLayoutData.isReplaceAllowed = result.isContentEditable && atBoundaryOfGranularity(selection.start(), WordGranularity, DirectionForward);
187210
postLayoutData.wordAtSelection = plainTextReplacingNoBreakSpace(wordRangeFromPosition(selection.start()).get());
188-
if (selection.isContentEditable()) {
211+
if (selection.isContentEditable())
189212
charactersAroundPosition(selection.start(), postLayoutData.characterAfterSelection, postLayoutData.characterBeforeSelection, postLayoutData.twoCharacterBeforeSelection);
190-
Node* root = selection.rootEditableElement();
191-
postLayoutData.hasContent = root && root->hasChildNodes() && !isEndOfEditableOrNonEditableContent(firstPositionInNode(root));
192-
}
193213
} else if (selection.isRange()) {
194214
postLayoutData.caretRectAtStart = view->contentsToRootView(VisiblePosition(selection.start()).absoluteCaretBounds(&startNodeIsInsideFixedPosition));
195215
postLayoutData.caretRectAtEnd = view->contentsToRootView(VisiblePosition(selection.end()).absoluteCaretBounds(&endNodeIsInsideFixedPosition));
@@ -211,6 +231,7 @@
211231
if (!selection.isNone()) {
212232
if (m_assistedNode && m_assistedNode->renderer())
213233
postLayoutData.selectionClipRect = view->contentsToRootView(m_assistedNode->renderer()->absoluteBoundingBoxRect());
234+
computeEditableRootHasContentAndPlainText(selection, postLayoutData);
214235
}
215236
}
216237

Tools/ChangeLog

+29
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,32 @@
1+
2017-09-29 Wenson Hsieh <[email protected]>
2+
3+
[iOS WK2] Implement -[WKContentView hasText] for compatibility with the UITextInput protocol
4+
https://bugs.webkit.org/show_bug.cgi?id=177662
5+
<rdar://problem/33410373>
6+
7+
Reviewed by Tim Horton.
8+
9+
Add EditorState API tests to check that the value of -[WKContentView hasText] is correct when editing both plain
10+
and rich text areas.
11+
12+
* TestWebKitAPI/EditingTestHarness.h:
13+
* TestWebKitAPI/EditingTestHarness.mm:
14+
(-[EditingTestHarness insertParagraph]):
15+
(-[EditingTestHarness insertText:]):
16+
(-[EditingTestHarness insertHTML:]):
17+
(-[EditingTestHarness selectAll]):
18+
(-[EditingTestHarness deleteBackwards]):
19+
* TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm:
20+
21+
Add versions of EditingTestHarness helpers that don't require us to expect any editor state after executing the
22+
edit command.
23+
24+
(TestWebKitAPI::checkContentViewHasTextWithFailureDescription):
25+
(TestWebKitAPI::TEST):
26+
* TestWebKitAPI/cocoa/TestWKWebView.h:
27+
* TestWebKitAPI/cocoa/TestWKWebView.mm:
28+
(-[TestWKWebView textInputContentView]):
29+
130
2017-09-29 Charles Turner <[email protected]>
231

332
Update my status.

Tools/TestWebKitAPI/EditingTestHarness.h

+5
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@
4141
@property (nonatomic, readonly) NSDictionary *latestEditorState;
4242
@property (nonatomic, readonly) NSArray<NSDictionary *> *editorStateHistory;
4343

44+
- (void)insertParagraph;
45+
- (void)insertText:(NSString *)text;
46+
- (void)insertHTML:(NSString *)html;
47+
- (void)selectAll;
48+
- (void)deleteBackwards;
4449
- (void)insertParagraphAndExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries;
4550
- (void)insertText:(NSString *)text andExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries;
4651
- (void)insertHTML:(NSString *)html andExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries;

Tools/TestWebKitAPI/EditingTestHarness.mm

+25
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,31 @@ - (NSDictionary *)latestEditorState
6767
return _editorStateHistory.get();
6868
}
6969

70+
- (void)insertParagraph
71+
{
72+
[self insertParagraphAndExpectEditorStateWith:nil];
73+
}
74+
75+
- (void)insertText:(NSString *)text
76+
{
77+
[self insertText:text andExpectEditorStateWith:nil];
78+
}
79+
80+
- (void)insertHTML:(NSString *)html
81+
{
82+
[self insertHTML:html andExpectEditorStateWith:nil];
83+
}
84+
85+
- (void)selectAll
86+
{
87+
[self selectAllAndExpectEditorStateWith:nil];
88+
}
89+
90+
- (void)deleteBackwards
91+
{
92+
[self deleteBackwardAndExpectEditorStateWith:nil];
93+
}
94+
7095
- (void)insertText:(NSString *)text andExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries
7196
{
7297
[self _execCommand:@"InsertText" argument:text expectEntries:entries];

Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm

+71
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@
3232
#import "TestWKWebView.h"
3333
#import <WebKit/WKWebViewPrivate.h>
3434

35+
#if PLATFORM(IOS)
36+
#import <UIKit/UIKit.h>
37+
#endif
38+
3539
namespace TestWebKitAPI {
3640

3741
static RetainPtr<EditingTestHarness> setUpEditorStateTestHarness()
@@ -229,6 +233,73 @@
229233
EXPECT_WK_STREQ("https://www.apple.com/", [[testHarness webView] stringByEvaluatingJavaScript:@"document.querySelector('a').href"]);
230234
}
231235

236+
#if PLATFORM(IOS)
237+
238+
static void checkContentViewHasTextWithFailureDescription(TestWKWebView *webView, BOOL expectedToHaveText, NSString *description)
239+
{
240+
BOOL hasText = webView.textInputContentView.hasText;
241+
if (expectedToHaveText)
242+
EXPECT_TRUE(hasText);
243+
else
244+
EXPECT_FALSE(hasText);
245+
246+
if (expectedToHaveText != hasText)
247+
NSLog(@"Expected -[%@ hasText] to be %@, but observed: %@ (%@)", [webView.textInputContentView class], expectedToHaveText ? @"YES" : @"NO", hasText ? @"YES" : @"NO", description);
248+
}
249+
250+
TEST(EditorStateTests, ContentViewHasTextInContentEditableElement)
251+
{
252+
auto testHarness = setUpEditorStateTestHarness();
253+
TestWKWebView *webView = [testHarness webView];
254+
255+
checkContentViewHasTextWithFailureDescription(webView, NO, @"before inserting any content");
256+
[testHarness insertHTML:@"<img src='icon.png'></img>"];
257+
checkContentViewHasTextWithFailureDescription(webView, NO, @"after inserting an image element");
258+
[testHarness insertText:@"A"];
259+
checkContentViewHasTextWithFailureDescription(webView, YES, @"after inserting text");
260+
[testHarness selectAll];
261+
checkContentViewHasTextWithFailureDescription(webView, YES, @"after selecting everything");
262+
[testHarness deleteBackwards];
263+
checkContentViewHasTextWithFailureDescription(webView, NO, @"after deleting everything");
264+
[testHarness insertParagraph];
265+
checkContentViewHasTextWithFailureDescription(webView, YES, @"after inserting a newline");
266+
[testHarness deleteBackwards];
267+
checkContentViewHasTextWithFailureDescription(webView, NO, @"after deleting the newline");
268+
[testHarness insertText:@"B"];
269+
checkContentViewHasTextWithFailureDescription(webView, YES, @"after inserting text again");
270+
[webView stringByEvaluatingJavaScript:@"document.body.blur()"];
271+
[webView waitForNextPresentationUpdate];
272+
checkContentViewHasTextWithFailureDescription(webView, NO, @"after losing focus");
273+
}
274+
275+
TEST(EditorStateTests, ContentViewHasTextInTextarea)
276+
{
277+
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
278+
auto testHarness = adoptNS([[EditingTestHarness alloc] initWithWebView:webView.get()]);
279+
[webView synchronouslyLoadHTMLString:@"<textarea id='textarea'></textarea>"];
280+
[webView stringByEvaluatingJavaScript:@"textarea.focus()"];
281+
[webView waitForNextPresentationUpdate];
282+
283+
checkContentViewHasTextWithFailureDescription(webView.get(), NO, @"before inserting any content");
284+
[testHarness insertText:@"A"];
285+
checkContentViewHasTextWithFailureDescription(webView.get(), YES, @"after inserting text");
286+
[testHarness selectAll];
287+
checkContentViewHasTextWithFailureDescription(webView.get(), YES, @"after selecting everything");
288+
[testHarness deleteBackwards];
289+
checkContentViewHasTextWithFailureDescription(webView.get(), NO, @"after deleting everything");
290+
[testHarness insertParagraph];
291+
checkContentViewHasTextWithFailureDescription(webView.get(), YES, @"after inserting a newline");
292+
[testHarness deleteBackwards];
293+
checkContentViewHasTextWithFailureDescription(webView.get(), NO, @"after deleting the newline");
294+
[testHarness insertText:@"B"];
295+
checkContentViewHasTextWithFailureDescription(webView.get(), YES, @"after inserting text again");
296+
[webView stringByEvaluatingJavaScript:@"textarea.blur()"];
297+
[webView waitForNextPresentationUpdate];
298+
checkContentViewHasTextWithFailureDescription(webView.get(), NO, @"after losing focus");
299+
}
300+
301+
#endif
302+
232303
} // namespace TestWebKitAPI
233304

234305
#endif // WK_API_ENABLED

Tools/TestWebKitAPI/cocoa/TestWKWebView.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@
2323
* THE POSSIBILITY OF SUCH DAMAGE.
2424
*/
2525

26+
#if WK_API_ENABLED
27+
2628
#import <WebKit/WebKit.h>
2729
#import <wtf/RetainPtr.h>
2830

29-
#if WK_API_ENABLED
30-
3131
@class _WKProcessPoolConfiguration;
3232

3333
#if PLATFORM(IOS)
@@ -53,6 +53,7 @@
5353

5454
#if PLATFORM(IOS)
5555
@interface TestWKWebView (IOSOnly)
56+
@property (nonatomic, readonly) UIView <UITextInput> *textInputContentView;
5657
@property (nonatomic, readonly) RetainPtr<NSArray> selectionRectsAfterPresentationUpdate;
5758
- (_WKActivatedElementInfo *)activatedElementAtPosition:(CGPoint)position;
5859
@end

Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

+6
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#endif
4646

4747
#if PLATFORM(IOS)
48+
#import <UIKit/UIKit.h>
4849
#import <wtf/SoftLinking.h>
4950
SOFT_LINK_FRAMEWORK(UIKit)
5051
SOFT_LINK_CLASS(UIKit, UIWindow)
@@ -292,6 +293,11 @@ - (void)waitForNextPresentationUpdate
292293

293294
@implementation TestWKWebView (IOSOnly)
294295

296+
- (UIView <UITextInput> *)textInputContentView
297+
{
298+
return (UIView <UITextInput> *)[self valueForKey:@"_currentContentView"];
299+
}
300+
295301
- (RetainPtr<NSArray>)selectionRectsAfterPresentationUpdate
296302
{
297303
RetainPtr<TestWKWebView> retainedSelf = self;

0 commit comments

Comments
 (0)