Attachment 359038 Details for Bug 193398 – Patch

[patch] Patch

bug-193398-20190114082751.patch (text/plain), 16.60 KB, created by Wenson Hsieh on 2019-01-14 08:27:51 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-01-14 08:27:51 PST

Size: 16.60 KB

patch

obsolete

>Subversion Revision: 239888
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 9f61e56ecbac2df797b801cebf5a1cc29f2c1778..47bb5deeefcd7400175dc62b614dae9affd88db7 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,37 @@
>+2019-01-14  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Expose SPI to access the current sentence boundary and selection state
>+        https://bugs.webkit.org/show_bug.cgi?id=193398
>+        <rdar://problem/45893108>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Expose SPI on WKWebView for internal clients to grab information about attributes at the current selection; so
>+        far, this only includes whether the selection is a caret or a range, and whether or not the start of the
>+        selection is at the start of a new sentence.
>+
>+        Test: EditorStateTests.ObserveSelectionAttributeChanges
>+
>+        * Shared/EditorState.cpp:
>+        (WebKit::EditorState::PostLayoutData::encode const):
>+        (WebKit::EditorState::PostLayoutData::decode):
>+        * Shared/EditorState.h:
>+
>+        Add a new bit in EditorState on iOS to compute whether or not the start of the selection is at the start of a
>+        new sentence. This is computed and set when sending post-layout data in `WebPageIOS.mm`.
>+
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (selectionAttributes):
>+        (-[WKWebView _didChangeEditorState]):
>+        (-[WKWebView _selectionAttributes]):
>+
>+        Make the new SPI property support KVO by invoking `-willChangeValueForKey:` and `-didChangeValueForKey:`
>+        whenever the selection attributes change.
>+
>+        * UIProcess/API/Cocoa/WKWebViewPrivate.h:
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::platformEditorState const):
>+
> 2019-01-11  John Wilander  <wilander@apple.com>
> 
>         Compile out Web API Statistics Collection
>diff --git a/Source/WebKit/Shared/EditorState.cpp b/Source/WebKit/Shared/EditorState.cpp
>index 896c03bede1a2c39d9c532814779eadb06cf7add..22b8b14791998a1515e46aa08adeb2ad83719780 100644
>--- a/Source/WebKit/Shared/EditorState.cpp
>+++ b/Source/WebKit/Shared/EditorState.cpp
>@@ -132,6 +132,7 @@ void EditorState::PostLayoutData::encode(IPC::Encoder& encoder) const
>     encoder << hasPlainText;
>     encoder << elementIsTransparentOrFullyClipped;
>     encoder << caretColor;
>+    encoder << atStartOfSentence;
> #endif
> #if PLATFORM(MAC)
>     encoder << candidateRequestStartPosition;
>@@ -191,6 +192,8 @@ bool EditorState::PostLayoutData::decode(IPC::Decoder& decoder, PostLayoutData&
>         return false;
>     if (!decoder.decode(result.caretColor))
>         return false;
>+    if (!decoder.decode(result.atStartOfSentence))
>+        return false;
> #endif
> #if PLATFORM(MAC)
>     if (!decoder.decode(result.candidateRequestStartPosition))
>diff --git a/Source/WebKit/Shared/EditorState.h b/Source/WebKit/Shared/EditorState.h
>index 0cc25cd8127e9d7ad3ee694d1d20f4827fcaa387..5ee4105ff79c9688a8cfcd321d63d17bd80017a8 100644
>--- a/Source/WebKit/Shared/EditorState.h
>+++ b/Source/WebKit/Shared/EditorState.h
>@@ -109,6 +109,7 @@ struct EditorState {
>         bool hasPlainText { false };
>         bool elementIsTransparentOrFullyClipped { false };
>         WebCore::Color caretColor;
>+        bool atStartOfSentence { false };
> #endif
> #if PLATFORM(MAC)
>         uint64_t candidateRequestStartPosition { 0 };
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index 11633c6b4d02fef7cab0d6a27eeb380c29f5f230..074cddfd2784e869128d020bed59213a1cbdde68 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -377,6 +377,7 @@ @implementation WKWebView {
>     std::unique_ptr<WebKit::WebViewImpl> _impl;
>     RetainPtr<WKTextFinderClient> _textFinderClient;
> #endif
>+    _WKSelectionAttributes _selectionAttributes;
>     CGFloat _minimumEffectiveDeviceWidth;
> }
> 
>@@ -1267,17 +1268,51 @@ static NSDictionary *dictionaryRepresentationForEditorState(const WebKit::Editor
>     };
> }
> 
>+static _WKSelectionAttributes selectionAttributes(const WebKit::EditorState& editorState, _WKSelectionAttributes previousAttributes)
>+{
>+    _WKSelectionAttributes attributes = _WKSelectionAttributeNoSelection;
>+    if (editorState.selectionIsNone)
>+        return attributes;
>+
>+    if (editorState.selectionIsRange)
>+        attributes |= _WKSelectionAttributeIsRange;
>+    else
>+        attributes |= _WKSelectionAttributeIsCaret;
>+
>+    if (!editorState.isMissingPostLayoutData) {
>+#if PLATFORM(IOS_FAMILY)
>+        if (editorState.postLayoutData().atStartOfSentence)
>+            attributes |= _WKSelectionAttributeAtStartOfSentence;
>+#endif
>+    } else if (previousAttributes & _WKSelectionAttributeAtStartOfSentence)
>+        attributes |= _WKSelectionAttributeAtStartOfSentence;
>+
>+    return attributes;
>+}
>+
> - (void)_didChangeEditorState
> {
>-    id <WKUIDelegatePrivate> uiDelegate = (id <WKUIDelegatePrivate>)self.UIDelegate;
>+    auto newSelectionAttributes = selectionAttributes(_page->editorState(), _selectionAttributes);
>+    if (_selectionAttributes != newSelectionAttributes) {
>+        NSString *selectionAttributesKey = NSStringFromSelector(@selector(_selectionAttributes));
>+        [self willChangeValueForKey:selectionAttributesKey];
>+        _selectionAttributes = newSelectionAttributes;
>+        [self didChangeValueForKey:selectionAttributesKey];
>+    }
> 
>     // FIXME: We should either rename -_webView:editorStateDidChange: to clarify that it's only intended for use when testing,
>     // or remove it entirely and use -_webView:didChangeFontAttributes: instead once text alignment is supported in the set of
>     // font attributes.
>+    id <WKUIDelegatePrivate> uiDelegate = (id <WKUIDelegatePrivate>)self.UIDelegate;
>     if ([uiDelegate respondsToSelector:@selector(_webView:editorStateDidChange:)])
>         [uiDelegate _webView:self editorStateDidChange:dictionaryRepresentationForEditorState(_page->editorState())];
> }
> 
>+- (_WKSelectionAttributes)_selectionAttributes
>+{
>+    return _selectionAttributes;
>+}
>+
> - (void)_showSafeBrowsingWarning:(const WebKit::SafeBrowsingWarning&)warning completionHandler:(CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, URL>&&)>&&)completionHandler
> {
>     _safeBrowsingWarning = adoptNS([[WKSafeBrowsingWarning alloc] initWithFrame:self.bounds safeBrowsingWarning:warning completionHandler:[weakSelf = WeakObjCPtr<WKWebView>(self), completionHandler = WTFMove(completionHandler)] (auto&& result) mutable {
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>index 29ae7935c35e0302cead50c9cfb63e505b909014..26c0c346002f58d4649cdad2c92d48d377becf22 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>@@ -63,6 +63,13 @@ typedef NS_OPTIONS(NSUInteger, _WKCaptureDevices) {
>     _WKCaptureDeviceDisplay = 1 << 2,
> } WK_API_AVAILABLE(macosx(10.13), ios(11.0));
> 
>+typedef NS_OPTIONS(NSUInteger, _WKSelectionAttributes) {
>+    _WKSelectionAttributeNoSelection = 0,
>+    _WKSelectionAttributeIsCaret = 1 << 0,
>+    _WKSelectionAttributeIsRange = 1 << 1,
>+    _WKSelectionAttributeAtStartOfSentence = 1 << 2,
>+} WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
>+
> #if TARGET_OS_IPHONE
> 
> typedef NS_ENUM(NSUInteger, _WKDragInteractionPolicy) {
>@@ -220,6 +227,7 @@ typedef NS_OPTIONS(NSUInteger, _WKRectEdge) {
> - (IBAction)_takeFindStringFromSelection:(id)sender WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> @property (class, nonatomic, copy, setter=_setStringForFind:) NSString *_stringForFind WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
>+@property (nonatomic, readonly) _WKSelectionAttributes _selectionAttributes WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> #if TARGET_OS_IPHONE
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 0947f06f45de66e5de841207263db7d62e2e5594..c74a636b0a32e95aa79b6ce124bb397313c7e50b 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -240,6 +240,7 @@ void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePost
>         // FIXME: We should disallow replace when the string contains only CJ characters.
>         postLayoutData.isReplaceAllowed = result.isContentEditable && !result.isInPasswordField && !selectedText.isAllSpecialCharacters<isHTMLSpace>();
>     }
>+    postLayoutData.atStartOfSentence = frame.selection().selectionAtSentenceStart();
>     postLayoutData.insideFixedPosition = startNodeIsInsideFixedPosition || endNodeIsInsideFixedPosition;
>     if (!selection.isNone()) {
>         if (m_focusedElement && m_focusedElement->renderer()) {
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 252239f5273c4652132a2366a016d2be15a6efed..16af078ea084ab19a47248072ccd269244318f75 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-01-14  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Expose SPI to access the current sentence boundary and selection state
>+        https://bugs.webkit.org/show_bug.cgi?id=193398
>+        <rdar://problem/45893108>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add an API test to verify that an SPI client can observe changes in the `@"_selectionAttributes"` key path on
>+        WKWebView, and that inserting text, deleting, and changing the selection cause selection attributes to change as
>+        expected.
>+
>+        * TestWebKitAPI/EditingTestHarness.h:
>+        * TestWebKitAPI/EditingTestHarness.mm:
>+        (-[EditingTestHarness moveBackward]):
>+        (-[EditingTestHarness moveForward]):
>+        (-[EditingTestHarness moveForwardAndExpectEditorStateWith:]):
>+
>+        Add a couple of new helper methods on EditingTestHarness.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm:
>+        (-[SelectionChangeObserver initWithWebView:]):
>+        (-[SelectionChangeObserver webView]):
>+        (-[SelectionChangeObserver observeValueForKeyPath:ofObject:change:context:]):
>+        (-[SelectionChangeObserver currentSelectionAttributes]):
>+
> 2019-01-11  John Wilander  <wilander@apple.com>
> 
>         Compile out Web API Statistics Collection
>diff --git a/Tools/TestWebKitAPI/EditingTestHarness.h b/Tools/TestWebKitAPI/EditingTestHarness.h
>index 29af32161c7530f26eb61ab1733317c1d1d13de7..2f05d9e7dc31dddd7d66e8dc74c888b5fd2d0f6d 100644
>--- a/Tools/TestWebKitAPI/EditingTestHarness.h
>+++ b/Tools/TestWebKitAPI/EditingTestHarness.h
>@@ -46,6 +46,8 @@
> - (void)insertHTML:(NSString *)html;
> - (void)selectAll;
> - (void)deleteBackwards;
>+- (void)moveBackward;
>+- (void)moveForward;
> - (void)insertParagraphAndExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries;
> - (void)insertText:(NSString *)text andExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries;
> - (void)insertHTML:(NSString *)html andExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries;
>diff --git a/Tools/TestWebKitAPI/EditingTestHarness.mm b/Tools/TestWebKitAPI/EditingTestHarness.mm
>index 92b1b9eeae85beab9ed859f72bfdce2ce609304c..0dc5277af0f27e93e475a1fce816f912c0d52ce1 100644
>--- a/Tools/TestWebKitAPI/EditingTestHarness.mm
>+++ b/Tools/TestWebKitAPI/EditingTestHarness.mm
>@@ -92,6 +92,16 @@ - (void)deleteBackwards
>     [self deleteBackwardAndExpectEditorStateWith:nil];
> }
> 
>+- (void)moveBackward
>+{
>+    [self moveBackwardAndExpectEditorStateWith:nil];
>+}
>+
>+- (void)moveForward
>+{
>+    [self moveForwardAndExpectEditorStateWith:nil];
>+}
>+
> - (void)insertText:(NSString *)text andExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries
> {
>     [self _execCommand:@"InsertText" argument:text expectEntries:entries];
>@@ -117,6 +127,11 @@ - (void)moveWordBackwardAndExpectEditorStateWith:(NSDictionary<NSString *, id> *
>     [self _execCommand:@"MoveWordBackward" argument:nil expectEntries:entries];
> }
> 
>+- (void)moveForwardAndExpectEditorStateWith:(NSDictionary<NSString *, id> *)entries
>+{
>+    [self _execCommand:@"MoveForward" argument:nil expectEntries:entries];
>+}
>+
> - (void)toggleBold
> {
>     [self _execCommand:@"ToggleBold" argument:nil expectEntries:nil];
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm
>index 146f9517509f3b1422aaf3c4f4150e8032e490cc..d787dcda9951a97b5951b8abc9d6ddb31cf67dc4 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm
>@@ -31,12 +31,59 @@
> #import "PlatformUtilities.h"
> #import "TestWKWebView.h"
> #import <WebKit/WKWebViewPrivate.h>
>+#import <wtf/Vector.h>
> 
> #if PLATFORM(IOS_FAMILY)
> #import "UIKitSPI.h"
> #import <UIKit/UIKit.h>
> #endif
> 
>+static void* const SelectionAttributesObservationContext = (void*)&SelectionAttributesObservationContext;
>+
>+@interface SelectionChangeObserver : NSObject
>+- (instancetype)initWithWebView:(TestWKWebView *)webView;
>+@property (nonatomic, readonly) TestWKWebView *webView;
>+@property (nonatomic, readonly) _WKSelectionAttributes currentSelectionAttributes;
>+@end
>+
>+@implementation SelectionChangeObserver {
>+    RetainPtr<TestWKWebView> _webView;
>+    Vector<_WKSelectionAttributes> _observedSelectionAttributes;
>+}
>+
>+- (instancetype)initWithWebView:(TestWKWebView *)webView
>+{
>+    if (!(self = [super init]))
>+        return nil;
>+
>+    _webView = webView;
>+    [_webView addObserver:self forKeyPath:@"_selectionAttributes" options:NSKeyValueObservingOptionOld | NSKeyValueObservingOptionNew context:SelectionAttributesObservationContext];
>+    return self;
>+}
>+
>+- (TestWKWebView *)webView
>+{
>+    return _webView.get();
>+}
>+
>+- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey, id> *)change context:(void *)context
>+{
>+    if (context == SelectionAttributesObservationContext) {
>+        if (!_observedSelectionAttributes.isEmpty())
>+            EXPECT_EQ(_observedSelectionAttributes.last(), [change[NSKeyValueChangeOldKey] unsignedIntValue]);
>+        _observedSelectionAttributes.append([change[NSKeyValueChangeNewKey] unsignedIntValue]);
>+        return;
>+    }
>+    [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
>+}
>+
>+- (_WKSelectionAttributes)currentSelectionAttributes
>+{
>+    return _observedSelectionAttributes.isEmpty() ? _WKSelectionAttributeNoSelection : _observedSelectionAttributes.last();
>+}
>+
>+@end
>+
> namespace TestWebKitAPI {
> 
> static RetainPtr<EditingTestHarness> setUpEditorStateTestHarness()
>@@ -312,7 +359,47 @@ TEST(EditorStateTests, CaretColorInContentEditable)
>     auto cgRedColor = adoptCF(CGColorCreateCopyByMatchingToColorSpace(colorSpace.get(), kCGRenderingIntentDefault, redColor.CGColor, NULL));
>     EXPECT_TRUE(CGColorEqualToColor(cgInsertionPointColor.get(), cgRedColor.get()));
> }
>-#endif
>+
>+TEST(EditorStateTests, ObserveSelectionAttributeChanges)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>+    auto editor = adoptNS([[EditingTestHarness alloc] initWithWebView:webView.get()]);
>+    [webView _setEditable:YES];
>+    [webView synchronouslyLoadHTMLString:@"<body></body>"];
>+
>+    auto observer = adoptNS([[SelectionChangeObserver alloc] initWithWebView:webView.get()]);
>+
>+    [webView evaluateJavaScript:@"document.body.focus()" completionHandler:nil];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(_WKSelectionAttributeIsCaret | _WKSelectionAttributeAtStartOfSentence, [observer currentSelectionAttributes]);
>+
>+    [editor insertText:@"Hello"];
>+    EXPECT_EQ(_WKSelectionAttributeIsCaret, [observer currentSelectionAttributes]);
>+
>+    [editor insertText:@"."];
>+    EXPECT_EQ(_WKSelectionAttributeIsCaret | _WKSelectionAttributeAtStartOfSentence, [observer currentSelectionAttributes]);
>+
>+    [editor moveBackward];
>+    EXPECT_EQ(_WKSelectionAttributeIsCaret, [observer currentSelectionAttributes]);
>+
>+    [editor moveForward];
>+    EXPECT_EQ(_WKSelectionAttributeIsCaret | _WKSelectionAttributeAtStartOfSentence, [observer currentSelectionAttributes]);
>+
>+    [editor deleteBackwards];
>+    EXPECT_EQ(_WKSelectionAttributeIsCaret, [observer currentSelectionAttributes]);
>+
>+    [editor insertParagraph];
>+    EXPECT_EQ(_WKSelectionAttributeIsCaret | _WKSelectionAttributeAtStartOfSentence, [observer currentSelectionAttributes]);
>+
>+    [editor selectAll];
>+    EXPECT_EQ(_WKSelectionAttributeIsRange | _WKSelectionAttributeAtStartOfSentence, [observer currentSelectionAttributes]);
>+
>+    [webView evaluateJavaScript:@"getSelection().removeAllRanges()" completionHandler:nil];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(_WKSelectionAttributeNoSelection, [observer currentSelectionAttributes]);
>+}
>+
>+#endif // PLATFORM(IOS_FAMILY)
> 
> } // namespace TestWebKitAPI
>

Actions: View | Formatted Diff | Diff

Attachments on bug 193398: 359038