Attachment 362147 Details for Bug 190015 – Patch for landing

[patch] Patch for landing

bug-190015-20190215124435.patch (text/plain), 14.84 KB, created by Wenson Hsieh on 2019-02-15 12:44:36 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-02-15 12:44:36 PST

Size: 14.84 KB

patch

obsolete

>Subversion Revision: 241602
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 7994b6f5d19b6e72ea534154ca1a28eb340165a8..8c508ac51adb521f7ba5f801144d63897b90c010 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,49 @@
>+2019-02-15  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] WKWebView callout bar is missing Change Writing Direction item
>+        https://bugs.webkit.org/show_bug.cgi?id=190015
>+        <rdar://problem/44810366>
>+
>+        Reviewed by Tim Horton.
>+
>+        Support -makeTextWritingDirectionLeftToRight: and -makeTextWritingDirectionRightToLeft: in WKWebView on iOS.
>+        To match behavior in native UITextViews on iOS, we implement these methods by changing the *base* writing
>+        direction, rather than the text writing direction (this is in contrast to macOS, which has different
>+        NSResponder methods for changing the base writing direction as opposed to the text writing direction).
>+
>+        Additionally fixes the implementation of -makeTextWritingDirectionNatural:, which currently attempts to change
>+        the text writing direction instead of the base writing direction.
>+
>+        * Platform/spi/ios/UIKitSPI.h:
>+
>+        Add a forward declaration for keyboards SPI to determine whether the user has an active RTL keyboard.
>+
>+        * Shared/EditorState.cpp:
>+        (WebKit::EditorState::PostLayoutData::encode const):
>+        (WebKit::EditorState::PostLayoutData::decode):
>+        (WebKit::operator<<):
>+        * Shared/EditorState.h:
>+
>+        Plumb the base writing direction to the UI process through EditorState.
>+
>+        * UIProcess/ios/WKContentViewInteraction.h:
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView makeTextWritingDirectionNaturalForWebView:]):
>+        (-[WKContentView makeTextWritingDirectionLeftToRightForWebView:]):
>+        (-[WKContentView makeTextWritingDirectionRightToLeftForWebView:]):
>+
>+        Implement the new SPI (see above for more details).
>+
>+        (-[WKContentView canPerformActionForWebView:withSender:]):
>+
>+        Implement -canPerformAction: for LTR and RTL actions. To match existing UIWebView behavior, we only enable
>+        these actions if either the base writing direction is RTL, or the user has an active RTL keyboard. This means,
>+        for instance, that in the case where a user with only an English keyboard is editing LTR content, we would never
>+        show an option to convert to RTL.
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::editorState const):
>+
> 2019-02-15  Brian Burg  <bburg@apple.com>
> 
>         [Mac] WebInspectorUI.framework does not need to be soft-linked anymore
>diff --git a/Source/WebKit/Platform/spi/ios/UIKitSPI.h b/Source/WebKit/Platform/spi/ios/UIKitSPI.h
>index 02c60018d4a45fa4696c82f62defe08c029e3f03..394ffe1dbd8f53d76da581a0d23142abf56e934d 100644
>--- a/Source/WebKit/Platform/spi/ios/UIKitSPI.h
>+++ b/Source/WebKit/Platform/spi/ios/UIKitSPI.h
>@@ -1143,6 +1143,7 @@ WTF_EXTERN_C_BEGIN
> 
> BOOL UIKeyboardEnabledInputModesAllowOneToManyShortcuts(void);
> BOOL UIKeyboardEnabledInputModesAllowChineseTransliterationForText(NSString *);
>+BOOL UIKeyboardIsRightToLeftInputModeActive(void);
> 
> extern const float UITableCellDefaultFontSize;
> extern const float UITableViewCellDefaultFontSize;
>diff --git a/Source/WebKit/Shared/EditorState.cpp b/Source/WebKit/Shared/EditorState.cpp
>index 22b8b14791998a1515e46aa08adeb2ad83719780..c1153b2863a50b97b046d25d93711de5dd3df203 100644
>--- a/Source/WebKit/Shared/EditorState.cpp
>+++ b/Source/WebKit/Shared/EditorState.cpp
>@@ -111,12 +111,13 @@ void EditorState::PostLayoutData::encode(IPC::Encoder& encoder) const
> #if PLATFORM(IOS_FAMILY) || PLATFORM(GTK)
>     encoder << caretRectAtStart;
> #endif
>-#if PLATFORM(IOS_FAMILY) || PLATFORM(MAC)
>+#if PLATFORM(COCOA)
>     encoder << focusedElementRect;
>     encoder << selectedTextLength;
>     encoder << textAlignment;
>     encoder << textColor;
>     encoder << enclosingListType;
>+    encoder << baseWritingDirection;
> #endif
> #if PLATFORM(IOS_FAMILY)
>     encoder << caretRectAtEnd;
>@@ -153,7 +154,7 @@ bool EditorState::PostLayoutData::decode(IPC::Decoder& decoder, PostLayoutData&
>     if (!decoder.decode(result.caretRectAtStart))
>         return false;
> #endif
>-#if PLATFORM(IOS_FAMILY) || PLATFORM(MAC)
>+#if PLATFORM(COCOA)
>     if (!decoder.decode(result.focusedElementRect))
>         return false;
>     if (!decoder.decode(result.selectedTextLength))
>@@ -164,6 +165,8 @@ bool EditorState::PostLayoutData::decode(IPC::Decoder& decoder, PostLayoutData&
>         return false;
>     if (!decoder.decode(result.enclosingListType))
>         return false;
>+    if (!decoder.decode(result.baseWritingDirection))
>+        return false;
> #endif
> #if PLATFORM(IOS_FAMILY)
>     if (!decoder.decode(result.caretRectAtEnd))
>@@ -264,7 +267,7 @@ TextStream& operator<<(TextStream& ts, const EditorState& editorState)
>     if (editorState.postLayoutData().caretRectAtStart != IntRect())
>         ts.dumpProperty("caretRectAtStart", editorState.postLayoutData().caretRectAtStart);
> #endif
>-#if PLATFORM(IOS_FAMILY) || PLATFORM(MAC)
>+#if PLATFORM(COCOA)
>     if (editorState.postLayoutData().focusedElementRect != IntRect())
>         ts.dumpProperty("focusedElementRect", editorState.postLayoutData().focusedElementRect);
>     if (editorState.postLayoutData().selectedTextLength)
>@@ -275,7 +278,9 @@ TextStream& operator<<(TextStream& ts, const EditorState& editorState)
>         ts.dumpProperty("textColor", editorState.postLayoutData().textColor);
>     if (editorState.postLayoutData().enclosingListType != NoList)
>         ts.dumpProperty("enclosingListType", editorState.postLayoutData().enclosingListType);
>-#endif
>+    if (editorState.postLayoutData().baseWritingDirection != WebCore::WritingDirection::Natural)
>+        ts.dumpProperty("baseWritingDirection", static_cast<uint8_t>(editorState.postLayoutData().baseWritingDirection));
>+#endif // PLATFORM(COCOA)
> #if PLATFORM(IOS_FAMILY)
>     if (editorState.postLayoutData().caretRectAtEnd != IntRect())
>         ts.dumpProperty("caretRectAtEnd", editorState.postLayoutData().caretRectAtEnd);
>diff --git a/Source/WebKit/Shared/EditorState.h b/Source/WebKit/Shared/EditorState.h
>index 5ee4105ff79c9688a8cfcd321d63d17bd80017a8..4e1d5a4eb04b561f29f8d50f965f006e0ebbcb4e 100644
>--- a/Source/WebKit/Shared/EditorState.h
>+++ b/Source/WebKit/Shared/EditorState.h
>@@ -29,6 +29,7 @@
> #include <WebCore/Color.h>
> #include <WebCore/FontAttributes.h>
> #include <WebCore/IntRect.h>
>+#include <WebCore/WritingDirection.h>
> #include <wtf/text/WTFString.h>
> 
> #if PLATFORM(IOS_FAMILY)
>@@ -88,12 +89,13 @@ struct EditorState {
> #if PLATFORM(IOS_FAMILY) || PLATFORM(GTK)
>         WebCore::IntRect caretRectAtStart;
> #endif
>-#if PLATFORM(IOS_FAMILY) || PLATFORM(MAC)
>+#if PLATFORM(COCOA)
>         WebCore::IntRect focusedElementRect;
>         uint64_t selectedTextLength { 0 };
>         uint32_t textAlignment { NoAlignment };
>         WebCore::Color textColor { WebCore::Color::black };
>         uint32_t enclosingListType { NoList };
>+        WebCore::WritingDirection baseWritingDirection { WebCore::WritingDirection::Natural };
> #endif
> #if PLATFORM(IOS_FAMILY)
>         WebCore::IntRect caretRectAtEnd;
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>index 2cc7d0079db944765b3c03d12c34f912edcbbe28..dd4cc3c9d3024999a3d75f1f726c8ea74016ece6 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>@@ -131,7 +131,9 @@ typedef std::pair<WebKit::InteractionInformationRequest, InteractionInformationC
>     M(increaseSize) \
>     M(decreaseSize) \
>     M(pasteAndMatchStyle) \
>-    M(makeTextWritingDirectionNatural)
>+    M(makeTextWritingDirectionNatural) \
>+    M(makeTextWritingDirectionLeftToRight) \
>+    M(makeTextWritingDirectionRightToLeft)
> 
> #define FOR_EACH_PRIVATE_WKCONTENTVIEW_ACTION(M) \
>     M(_alignCenter) \
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index ae3485299a093b79fbebb3b63342418967477f96..e7587403c96aae3d9a5490240326e864f504b688 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -2471,7 +2471,19 @@ - (NSString *)selectedText
> 
> - (void)makeTextWritingDirectionNaturalForWebView:(id)sender
> {
>-    _page->executeEditCommand("makeTextWritingDirectionNatural"_s);
>+    // Match platform behavior on iOS as well as legacy WebKit behavior by modifying the
>+    // base (paragraph) writing direction rather than the inline direction.
>+    _page->setBaseWritingDirection(WebCore::WritingDirection::Natural);
>+}
>+
>+- (void)makeTextWritingDirectionLeftToRightForWebView:(id)sender
>+{
>+    _page->setBaseWritingDirection(WebCore::WritingDirection::LeftToRight);
>+}
>+
>+- (void)makeTextWritingDirectionRightToLeftForWebView:(id)sender
>+{
>+    _page->setBaseWritingDirection(WebCore::WritingDirection::RightToLeft);
> }
> 
> - (BOOL)isReplaceAllowed
>@@ -2798,6 +2810,24 @@ - (BOOL)canPerformActionForWebView:(SEL)action withSender:(id)sender
>     if (action == @selector(replace:))
>         return editorState.isContentEditable && !editorState.isInPasswordField;
> 
>+    if (action == @selector(makeTextWritingDirectionLeftToRight:) || action == @selector(makeTextWritingDirectionRightToLeft:)) {
>+        if (!editorState.isContentEditable)
>+            return NO;
>+
>+        auto baseWritingDirection = editorState.postLayoutData().baseWritingDirection;
>+        if (baseWritingDirection == WebCore::WritingDirection::LeftToRight && !UIKeyboardIsRightToLeftInputModeActive()) {
>+            // A keyboard is considered "active" if it is available for the user to switch to. As such, this check prevents
>+            // text direction actions from showing up in the case where a user has only added left-to-right keyboards, and
>+            // is also not editing right-to-left content.
>+            return NO;
>+        }
>+
>+        if (action == @selector(makeTextWritingDirectionLeftToRight:))
>+            return baseWritingDirection != WebCore::WritingDirection::LeftToRight;
>+
>+        return baseWritingDirection != WebCore::WritingDirection::RightToLeft;
>+    }
>+
>     return [super canPerformAction:action withSender:sender];
> }
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index e5186c4d6adb74803c7a3347e20ed0c23e8b839b..c932b51625939c99cfd6a980fffab3218c94d87c 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -1022,6 +1022,8 @@ EditorState WebPage::editorState(IncludePostLayoutDataHint shouldIncludePostLayo
>                 else
>                     ASSERT_NOT_REACHED();
>             }
>+
>+            postLayoutData.baseWritingDirection = editor.baseWritingDirectionForSelectionStart();
>         }
> #endif
>     }
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index cbeaba2741cbb0d89f248c52b79443e90d950c68..9f6aad3b5fc7dda7acd20a55b0669bb0d6eb24b8 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-02-15  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] WKWebView callout bar is missing Change Writing Direction item
>+        https://bugs.webkit.org/show_bug.cgi?id=190015
>+        <rdar://problem/44810366>
>+
>+        Reviewed by Tim Horton.
>+
>+        Make an existing API test that exercises platform SPI to change the inline text writing direction run only on
>+        macOS, and add a new API test that uses similarly named SPI on iOS to change the base writing direction.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm:
>+        (TestWebKitAPI::TEST):
>+
> 2019-02-15  Youenn Fablet  <youenn@apple.com>
> 
>         Make navigator.mediaDevices SecureContext
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm
>index 1019482054f8a38d601b9c7aeab0cf166057cfef..9d7bfe4e2e493f0a117dfc4c5bdd3073c5cf8382 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm
>@@ -258,16 +258,32 @@ TEST(WKWebViewEditActions, PasteAndMatchStyle)
>     EXPECT_WK_STREQ("WebKit", [destination stringByEvaluatingJavaScript:@"getSelection().toString()"]);
> }
> 
>-TEST(WKWebViewEditActions, ModifyTextWritingDirection)
>+#if PLATFORM(IOS_FAMILY)
>+
>+TEST(WKWebViewEditActions, ModifyBaseWritingDirection)
> {
>-    auto webView = webViewForEditActionTesting(@"<div id='text' style='direction: rtl; unicode-bidi: bidi-override;'>WebKit</div>");
>-    [webView selectAll:nil];
>+    auto webView = webViewForEditActionTesting(@"<meta charset='utf8'><p id='english'>Hello world</p><p id='hebrew'>××§××¨ ××©× ×¢××¨××ª</p>");
>+
>+    [webView evaluateJavaScript:@"getSelection().setPosition(english)" completionHandler:nil];
>+    [webView makeTextWritingDirectionRightToLeft:nil];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_WK_STREQ("rtl", [webView stringByEvaluatingJavaScript:@"getComputedStyle(english).direction"]);
>+    EXPECT_FALSE([webView canPerformAction:@selector(makeTextWritingDirectionRightToLeft:) withSender:nil]);
>+    EXPECT_TRUE([webView canPerformAction:@selector(makeTextWritingDirectionLeftToRight:) withSender:nil]);
>+
>+    [webView evaluateJavaScript:@"getSelection().setPosition(hebrew)" completionHandler:nil];
>+    [webView makeTextWritingDirectionLeftToRight:nil];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_WK_STREQ("ltr", [webView stringByEvaluatingJavaScript:@"getComputedStyle(hebrew).direction"]);
>+    EXPECT_FALSE([webView canPerformAction:@selector(makeTextWritingDirectionLeftToRight:) withSender:nil]);
>+
>+    [webView evaluateJavaScript:@"getSelection().setPosition(english)" completionHandler:nil];
>     [webView makeTextWritingDirectionNatural:nil];
>-    EXPECT_WK_STREQ("normal", [webView stringByEvaluatingJavaScript:@"getComputedStyle(text).unicodeBidi"]);
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_WK_STREQ("ltr", [webView stringByEvaluatingJavaScript:@"getComputedStyle(english).direction"]);
>+    EXPECT_FALSE([webView canPerformAction:@selector(makeTextWritingDirectionLeftToRight:) withSender:nil]);
> }
> 
>-#if PLATFORM(IOS_FAMILY)
>-
> TEST(WKWebViewEditActions, ChangeFontSize)
> {
>     auto webView = webViewForEditActionTesting();
>@@ -328,7 +344,17 @@ TEST(WKWebViewEditActions, SetFontFamily)
>     EXPECT_WK_STREQ("italic", [webView stylePropertyAtSelectionStart:@"font-style"]);
> }
> 
>-#endif // PLATFORM(IOS_FAMILY)
>+#else
>+
>+TEST(WKWebViewEditActions, ModifyTextWritingDirection)
>+{
>+    auto webView = webViewForEditActionTesting(@"<div id='text' style='direction: rtl; unicode-bidi: bidi-override;'>WebKit</div>");
>+    [webView selectAll:nil];
>+    [webView makeTextWritingDirectionNatural:nil];
>+    EXPECT_WK_STREQ("normal", [webView stringByEvaluatingJavaScript:@"getComputedStyle(text).unicodeBidi"]);
>+}
>+
>+#endif
> 
> } // namespace TestWebKitAPI
>

Actions: View | Formatted Diff | Diff

Attachments on bug 190015: 362138 | 362147