Input type "formatSetInlineTextDirection" is dispatched when changing paragraph-level...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Feb 2019 17:09:27 +0000 (17:09 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Feb 2019 17:09:27 +0000 (17:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194703
<rdar://problem/48111775>

Reviewed by Ryosuke Niwa.

Source/WebCore:

Currently, when changing text direction, WebKit always sends input events of type formatSetInlineTextDirection,
even when changing paragraph text direction. Instead, we should be emitting formatSetBlockTextDirection in this
scenario. This is problematic when using the context menus on macOS to change writing direction, since changing
"Selection Direction" is currently indistinguishable from changing "Paragraph Direction".

To fix this, we split EditAction::SetWritingDirection into EditAction::SetInlineWritingDirection and
EditAction::SetBlockWritingDirection, which emit inline and block text direction input events, respectively.

Tests: fast/events/before-input-events-prevent-block-text-direction.html
       fast/events/before-input-events-prevent-inline-text-direction.html

* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::apply):
* editing/EditAction.cpp:
(WebCore::undoRedoLabel):
* editing/EditAction.h:
* editing/EditCommand.cpp:
(WebCore::inputTypeNameForEditingAction):
* editing/Editor.cpp:
(WebCore::inputEventDataForEditingStyleAndAction):
(WebCore::Editor::setBaseWritingDirection):
* editing/EditorCommand.cpp:
(WebCore::executeMakeTextWritingDirectionLeftToRight):
(WebCore::executeMakeTextWritingDirectionNatural):
(WebCore::executeMakeTextWritingDirectionRightToLeft):

Source/WebKitLegacy/win:

* WebCoreSupport/WebEditorClient.cpp:
(undoNameForEditAction):

LayoutTests:

Rebaseline some existing tests to expect input events of type "formatSetBlockTextDirection" instead of
"formatSetInlineTextDirection" when changing paragraph text direction; additionally, add a new layout test that
changes the inline text direction in some Bidi text, and verify that "formatSetInlineTextDirection" is emitted
in this scenario, and that calling `preventDefault()` in the beforeinput event handler causes no change to be
made.

* editing/input/ios/rtl-keyboard-input-on-focus-expected.txt:
* fast/events/before-input-events-prevent-block-text-direction-expected.txt: Added.
* fast/events/before-input-events-prevent-block-text-direction.html: Renamed from LayoutTests/fast/events/before-input-events-prevent-text-direction.html.
* fast/events/before-input-events-prevent-inline-text-direction-expected.txt: Added.
* fast/events/before-input-events-prevent-inline-text-direction.html: Added.
* fast/events/before-input-events-prevent-text-direction-expected.txt: Removed.

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/input/ios/rtl-keyboard-input-on-focus-expected.txt
LayoutTests/fast/events/before-input-events-prevent-block-text-direction-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/before-input-events-prevent-block-text-direction.html [moved from LayoutTests/fast/events/before-input-events-prevent-text-direction.html with 100% similarity]
LayoutTests/fast/events/before-input-events-prevent-inline-text-direction-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/before-input-events-prevent-inline-text-direction.html [new file with mode: 0644]
LayoutTests/fast/events/before-input-events-prevent-text-direction-expected.txt [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/editing/CompositeEditCommand.cpp
Source/WebCore/editing/EditAction.cpp
Source/WebCore/editing/EditAction.h
Source/WebCore/editing/EditCommand.cpp
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/EditorCommand.cpp
Source/WebKitLegacy/win/ChangeLog
Source/WebKitLegacy/win/WebCoreSupport/WebEditorClient.cpp

index 6756494..929b955 100644 (file)
@@ -1,3 +1,24 @@
+2019-02-22  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Input type "formatSetInlineTextDirection" is dispatched when changing paragraph-level text direction
+        https://bugs.webkit.org/show_bug.cgi?id=194703
+        <rdar://problem/48111775>
+
+        Reviewed by Ryosuke Niwa.
+
+        Rebaseline some existing tests to expect input events of type "formatSetBlockTextDirection" instead of
+        "formatSetInlineTextDirection" when changing paragraph text direction; additionally, add a new layout test that
+        changes the inline text direction in some Bidi text, and verify that "formatSetInlineTextDirection" is emitted
+        in this scenario, and that calling `preventDefault()` in the beforeinput event handler causes no change to be
+        made.
+
+        * editing/input/ios/rtl-keyboard-input-on-focus-expected.txt:
+        * fast/events/before-input-events-prevent-block-text-direction-expected.txt: Added.
+        * fast/events/before-input-events-prevent-block-text-direction.html: Renamed from LayoutTests/fast/events/before-input-events-prevent-text-direction.html.
+        * fast/events/before-input-events-prevent-inline-text-direction-expected.txt: Added.
+        * fast/events/before-input-events-prevent-inline-text-direction.html: Added.
+        * fast/events/before-input-events-prevent-text-direction-expected.txt: Removed.
+
 2019-02-22  Rob Buis  <rbuis@igalia.com>
 
         Fix unitless usage of mathsize
index 30b1950..fdc5538 100644 (file)
@@ -14,24 +14,24 @@ Using English keyboard:
 
 Observed 'beforeinput' events: [
     {
-        "inputType": "formatSetInlineTextDirection",
+        "inputType": "formatSetBlockTextDirection",
         "data": "rtl",
         "order": 1
     },
     {
-        "inputType": "formatSetInlineTextDirection",
+        "inputType": "formatSetBlockTextDirection",
         "data": "ltr",
         "order": 3
     }
 ]
 Observed 'input' events: [
     {
-        "inputType": "formatSetInlineTextDirection",
+        "inputType": "formatSetBlockTextDirection",
         "data": "rtl",
         "order": 2
     },
     {
-        "inputType": "formatSetInlineTextDirection",
+        "inputType": "formatSetBlockTextDirection",
         "data": "ltr",
         "order": 4
     }
diff --git a/LayoutTests/fast/events/before-input-events-prevent-block-text-direction-expected.txt b/LayoutTests/fast/events/before-input-events-prevent-block-text-direction-expected.txt
new file mode 100644 (file)
index 0000000..eb9e573
--- /dev/null
@@ -0,0 +1,17 @@
+Hello world
+
+*** TESTING RICH TEXT ***
+Initial text direction: "rtl"
+Fired onbeforeinput event of inputType 'formatSetBlockTextDirection' with data: 'ltr'
+Text direction after setting to LTR while preventing default: "rtl"
+Fired onbeforeinput event of inputType 'formatSetBlockTextDirection' with data: 'ltr'
+Fired input event of inputType 'formatSetBlockTextDirection' with data: 'ltr'
+Text direction after setting to LTR without preventing default: "ltr"
+*** TESTING PLAIN TEXT ***
+Initial text direction: "rtl"
+Fired onbeforeinput event of inputType 'formatSetBlockTextDirection' with data: 'ltr'
+Text direction after setting to LTR while preventing default: "rtl"
+Fired onbeforeinput event of inputType 'formatSetBlockTextDirection' with data: 'ltr'
+Fired input event of inputType 'formatSetBlockTextDirection' with data: 'ltr'
+Text direction after setting to LTR without preventing default: "ltr"
+
diff --git a/LayoutTests/fast/events/before-input-events-prevent-inline-text-direction-expected.txt b/LayoutTests/fast/events/before-input-events-prevent-inline-text-direction-expected.txt
new file mode 100644 (file)
index 0000000..8f996dd
--- /dev/null
@@ -0,0 +1,34 @@
+Verifies that changing the text writing direction fires input events which may be prevented. To manually test, select 'מקור' in the top contenteditable and change the selection direction to 'left to right'; then, select the same word in the prevented contenteditable and change selection direction to 'left to right'. Check that the bottom editable area's contents did not change.
+
+beforeinput, formatSetInlineTextDirection, ltr on #notPrevented:
+| "Hello <#selection-anchor>מקור<#selection-focus> השם עברית world"
+
+input, formatSetInlineTextDirection, ltr on #notPrevented:
+| "Hello "
+| <span>
+|   style="unicode-bidi: embed;"
+|   "<#selection-anchor>מקור<#selection-focus>"
+| " השם עברית world"
+
+beforeinput, formatSetInlineTextDirection, rtl on #notPrevented:
+| "Hello "
+| <span>
+|   style="unicode-bidi: embed;"
+|   "מקור"
+| " השם עברית <#selection-anchor>world<#selection-focus>"
+
+input, formatSetInlineTextDirection, rtl on #notPrevented:
+| "Hello "
+| <span>
+|   style="unicode-bidi: embed;"
+|   "מקור"
+| " השם עברית "
+| <span>
+|   style="unicode-bidi: embed; direction: rtl;"
+|   "<#selection-anchor>world<#selection-focus>"
+
+beforeinput, formatSetInlineTextDirection, ltr on #prevented:
+| "Hello <#selection-anchor>מקור<#selection-focus> השם עברית world"
+
+beforeinput, formatSetInlineTextDirection, rtl on #prevented:
+| "Hello מקור השם עברית <#selection-anchor>world<#selection-focus>"
diff --git a/LayoutTests/fast/events/before-input-events-prevent-inline-text-direction.html b/LayoutTests/fast/events/before-input-events-prevent-inline-text-direction.html
new file mode 100644 (file)
index 0000000..c536f0d
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<meta charset="utf8">
+<html>
+<head>
+    <script src="../../resources/dump-as-markup.js"></script>
+    <style>
+        #editor {
+            border: 1px blue dashed;
+        }
+    </style>
+    <script>
+        const dumpMarkupOnInput = e => Markup.dump(e.target, `${e.type}, ${e.inputType}, ${e.data} on #${e.target.id}`);
+
+        Markup.description("Verifies that changing the text writing direction fires input events which may be prevented. To manually test, select 'מקור' in the top contenteditable and change the selection direction to 'left to right'; then, select the same word in the prevented contenteditable and change selection direction to 'left to right'. Check that the bottom editable area's contents did not change.");
+        Markup.noAutoDump();
+
+        addEventListener("load", () => {
+            const notPrevented = document.querySelector("#notPrevented");
+            const prevented = document.querySelector("#prevented");
+
+            notPrevented.addEventListener("input", dumpMarkupOnInput);
+            notPrevented.addEventListener("beforeinput", dumpMarkupOnInput);
+            prevented.addEventListener("input", dumpMarkupOnInput);
+            prevented.addEventListener("beforeinput", event => {
+                dumpMarkupOnInput(event);
+                event.preventDefault();
+            });
+
+            getSelection().setBaseAndExtent(notPrevented.firstChild, 6, notPrevented.firstChild, 10);
+            if (!window.testRunner)
+                return;
+            testRunner.execCommand("MakeTextWritingDirectionLeftToRight");
+            getSelection().setBaseAndExtent(notPrevented.lastChild, 11, notPrevented.lastChild, 16);
+            testRunner.execCommand("MakeTextWritingDirectionRightToLeft");
+
+            getSelection().setBaseAndExtent(prevented.firstChild, 6, prevented.firstChild, 10);
+            testRunner.execCommand("MakeTextWritingDirectionLeftToRight");
+            getSelection().setBaseAndExtent(prevented.lastChild, 21, prevented.lastChild, 26);
+            testRunner.execCommand("MakeTextWritingDirectionRightToLeft");
+
+            Markup.notifyDone();
+        });
+    </script>
+</head>
+<body>
+    <div contenteditable id="notPrevented">Hello מקור השם עברית world</div>
+    <div contenteditable id="prevented">Hello מקור השם עברית world</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/events/before-input-events-prevent-text-direction-expected.txt b/LayoutTests/fast/events/before-input-events-prevent-text-direction-expected.txt
deleted file mode 100644 (file)
index 3620d7f..0000000
+++ /dev/null
@@ -1,17 +0,0 @@
-Hello world
-
-*** TESTING RICH TEXT ***
-Initial text direction: "rtl"
-Fired onbeforeinput event of inputType 'formatSetInlineTextDirection' with data: 'ltr'
-Text direction after setting to LTR while preventing default: "rtl"
-Fired onbeforeinput event of inputType 'formatSetInlineTextDirection' with data: 'ltr'
-Fired input event of inputType 'formatSetInlineTextDirection' with data: 'ltr'
-Text direction after setting to LTR without preventing default: "ltr"
-*** TESTING PLAIN TEXT ***
-Initial text direction: "rtl"
-Fired onbeforeinput event of inputType 'formatSetInlineTextDirection' with data: 'ltr'
-Text direction after setting to LTR while preventing default: "rtl"
-Fired onbeforeinput event of inputType 'formatSetInlineTextDirection' with data: 'ltr'
-Fired input event of inputType 'formatSetInlineTextDirection' with data: 'ltr'
-Text direction after setting to LTR without preventing default: "ltr"
-
index 067363d..8e793d0 100644 (file)
@@ -1,3 +1,37 @@
+2019-02-22  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Input type "formatSetInlineTextDirection" is dispatched when changing paragraph-level text direction
+        https://bugs.webkit.org/show_bug.cgi?id=194703
+        <rdar://problem/48111775>
+
+        Reviewed by Ryosuke Niwa.
+
+        Currently, when changing text direction, WebKit always sends input events of type formatSetInlineTextDirection,
+        even when changing paragraph text direction. Instead, we should be emitting formatSetBlockTextDirection in this
+        scenario. This is problematic when using the context menus on macOS to change writing direction, since changing
+        "Selection Direction" is currently indistinguishable from changing "Paragraph Direction".
+
+        To fix this, we split EditAction::SetWritingDirection into EditAction::SetInlineWritingDirection and
+        EditAction::SetBlockWritingDirection, which emit inline and block text direction input events, respectively.
+
+        Tests: fast/events/before-input-events-prevent-block-text-direction.html
+               fast/events/before-input-events-prevent-inline-text-direction.html
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::apply):
+        * editing/EditAction.cpp:
+        (WebCore::undoRedoLabel):
+        * editing/EditAction.h:
+        * editing/EditCommand.cpp:
+        (WebCore::inputTypeNameForEditingAction):
+        * editing/Editor.cpp:
+        (WebCore::inputEventDataForEditingStyleAndAction):
+        (WebCore::Editor::setBaseWritingDirection):
+        * editing/EditorCommand.cpp:
+        (WebCore::executeMakeTextWritingDirectionLeftToRight):
+        (WebCore::executeMakeTextWritingDirectionNatural):
+        (WebCore::executeMakeTextWritingDirectionRightToLeft):
+
 2019-02-22  Rob Buis  <rbuis@igalia.com>
 
         Remove stripLeadingAndTrailingWhitespace from MathMLElement.cpp
index 91d8af1..70e8ece 100644 (file)
@@ -342,7 +342,8 @@ void CompositeEditCommand::apply()
         case EditAction::TypingInsertFinalComposition:
         case EditAction::Paste:
         case EditAction::DeleteByDrag:
-        case EditAction::SetWritingDirection:
+        case EditAction::SetInlineWritingDirection:
+        case EditAction::SetBlockWritingDirection:
         case EditAction::Cut:
         case EditAction::Unspecified:
         case EditAction::Insert:
index 31d216f..733b429 100644 (file)
@@ -74,7 +74,8 @@ String undoRedoLabel(EditAction editAction)
         return WEB_UI_STRING_KEY("Center", "Center (Undo action name)", "Undo action name");
     case EditAction::Justify:
         return WEB_UI_STRING_KEY("Justify", "Justify (Undo action name)", "Undo action name");
-    case EditAction::SetWritingDirection:
+    case EditAction::SetInlineWritingDirection:
+    case EditAction::SetBlockWritingDirection:
         return WEB_UI_STRING_KEY("Set Writing Direction", "Set Writing Direction (Undo action name)", "Undo action name");
     case EditAction::Subscript:
         return WEB_UI_STRING_KEY("Subscript", "Subscript (Undo action name)", "Undo action name");
index 17e8be9..122bb4d 100644 (file)
@@ -54,7 +54,8 @@ enum class EditAction : uint8_t {
     AlignRight,
     Center,
     Justify,
-    SetWritingDirection,
+    SetInlineWritingDirection,
+    SetBlockWritingDirection,
     Subscript,
     Superscript,
     Underline,
index 34c79c9..2befe1f 100644 (file)
@@ -110,8 +110,10 @@ String inputTypeNameForEditingAction(EditAction action)
         return "formatIndent"_s;
     case EditAction::Outdent:
         return "formatOutdent"_s;
-    case EditAction::SetWritingDirection:
+    case EditAction::SetInlineWritingDirection:
         return "formatSetInlineTextDirection"_s;
+    case EditAction::SetBlockWritingDirection:
+        return "formatSetBlockTextDirection"_s;
     default:
         return emptyString();
     }
index 937e744..0a33f95 100644 (file)
@@ -153,7 +153,8 @@ static String inputEventDataForEditingStyleAndAction(const StyleProperties* styl
     switch (action) {
     case EditAction::SetColor:
         return style->getPropertyValue(CSSPropertyColor);
-    case EditAction::SetWritingDirection:
+    case EditAction::SetInlineWritingDirection:
+    case EditAction::SetBlockWritingDirection:
         return style->getPropertyValue(CSSPropertyDirection);
     default:
         return { };
@@ -1803,7 +1804,7 @@ void Editor::setBaseWritingDirection(WritingDirection direction)
 
         auto& focusedFormElement = downcast<HTMLTextFormControlElement>(*focusedElement);
         auto directionValue = direction == WritingDirection::LeftToRight ? "ltr" : "rtl";
-        auto writingDirectionInputTypeName = inputTypeNameForEditingAction(EditAction::SetWritingDirection);
+        auto writingDirectionInputTypeName = inputTypeNameForEditingAction(EditAction::SetBlockWritingDirection);
         if (!dispatchBeforeInputEvent(focusedFormElement, writingDirectionInputTypeName, directionValue))
             return;
 
@@ -1815,7 +1816,7 @@ void Editor::setBaseWritingDirection(WritingDirection direction)
 
     auto style = MutableStyleProperties::create();
     style->setProperty(CSSPropertyDirection, direction == WritingDirection::LeftToRight ? "ltr" : direction == WritingDirection::RightToLeft ? "rtl" : "inherit", false);
-    applyParagraphStyleToSelection(style.ptr(), EditAction::SetWritingDirection);
+    applyParagraphStyleToSelection(style.ptr(), EditAction::SetBlockWritingDirection);
 }
 
 WritingDirection Editor::baseWritingDirectionForSelectionStart() const
index b460b94..82163e6 100644 (file)
@@ -585,7 +585,7 @@ static bool executeMakeTextWritingDirectionLeftToRight(Frame& frame, Event*, Edi
     auto style = MutableStyleProperties::create();
     style->setProperty(CSSPropertyUnicodeBidi, CSSValueEmbed);
     style->setProperty(CSSPropertyDirection, CSSValueLtr);
-    frame.editor().applyStyle(style.ptr(), EditAction::SetWritingDirection);
+    frame.editor().applyStyle(style.ptr(), EditAction::SetInlineWritingDirection);
     return true;
 }
 
@@ -593,7 +593,7 @@ static bool executeMakeTextWritingDirectionNatural(Frame& frame, Event*, EditorC
 {
     auto style = MutableStyleProperties::create();
     style->setProperty(CSSPropertyUnicodeBidi, CSSValueNormal);
-    frame.editor().applyStyle(style.ptr(), EditAction::SetWritingDirection);
+    frame.editor().applyStyle(style.ptr(), EditAction::SetInlineWritingDirection);
     return true;
 }
 
@@ -602,7 +602,7 @@ static bool executeMakeTextWritingDirectionRightToLeft(Frame& frame, Event*, Edi
     auto style = MutableStyleProperties::create();
     style->setProperty(CSSPropertyUnicodeBidi, CSSValueEmbed);
     style->setProperty(CSSPropertyDirection, CSSValueRtl);
-    frame.editor().applyStyle(style.ptr(), EditAction::SetWritingDirection);
+    frame.editor().applyStyle(style.ptr(), EditAction::SetInlineWritingDirection);
     return true;
 }
 
index 3b78c27..9ad8227 100644 (file)
@@ -1,3 +1,14 @@
+2019-02-22  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Input type "formatSetInlineTextDirection" is dispatched when changing paragraph-level text direction
+        https://bugs.webkit.org/show_bug.cgi?id=194703
+        <rdar://problem/48111775>
+
+        Reviewed by Ryosuke Niwa.
+
+        * WebCoreSupport/WebEditorClient.cpp:
+        (undoNameForEditAction):
+
 2019-02-18  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS] Support pasting item-provider-backed data on the pasteboard as attachment elements
index 2e89d41..aae1e00 100644 (file)
@@ -588,7 +588,9 @@ static String undoNameForEditAction(EditAction editAction)
     case EditAction::AlignRight: return WEB_UI_STRING_KEY("Align Right", "Align Right (Undo action name)", "Undo action name");
     case EditAction::Center: return WEB_UI_STRING_KEY("Center", "Center (Undo action name)", "Undo action name");
     case EditAction::Justify: return WEB_UI_STRING_KEY("Justify", "Justify (Undo action name)", "Undo action name");
-    case EditAction::SetWritingDirection: return WEB_UI_STRING_KEY("Set Writing Direction", "Set Writing Direction (Undo action name)", "Undo action name");
+    case EditAction::SetInlineWritingDirection:
+    case EditAction::SetBlockWritingDirection:
+        return WEB_UI_STRING_KEY("Set Writing Direction", "Set Writing Direction (Undo action name)", "Undo action name");
     case EditAction::Subscript: return WEB_UI_STRING_KEY("Subscript", "Subscript (Undo action name)", "Undo action name");
     case EditAction::Superscript: return WEB_UI_STRING_KEY("Superscript", "Superscript (Undo action name)", "Undo action name");
     case EditAction::Bold: return WEB_UI_STRING_KEY("Bold", "Bold (Undo action name)", "Undo action name");