Fixed selection intents to take into account platform behavior

On Windows, moving forward by one word should move to the start of the next word, punctuation or line break.
On other platforms, it should first try to move to the end of the current word.
This patch clarifies the attached accessibility event intents to specify whether we
are moving to the end of the current word or to the start of the next one when selecting forward.

Also, this patch fixes the "sentence/line/paragraphboundary" granularities to
expose more information on the boundary they move to, depending on the selection direction.
It fixes the "sentence/line/paragraph" granularities to always indicate that they move to the start of the
next / previous boundary.

AX-Relnotes: n/a.

R=dmazzoni@chromium.org

Change-Id: I3298f0d73b6bb5d123a15be34fc8c6c587d9594c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2238186Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776835}

third_party/blink/renderer/core/accessibility/blink_ax_event_intent.cc

@@ -24,7 +24,8 @@ BlinkAXEventIntent BlinkAXEventIntent::FromModifiedSelection(
     const SelectionModifyDirection direction,
     const TextGranularity granularity,
     const SetSelectionBy set_selection_by,
-    const TextDirection direction_of_selection) {
+    const TextDirection direction_of_selection,
+    const PlatformWordBehavior platform_word_behavior) {
   ax::mojom::blink::Command command;
   switch (alter) {
     case SelectionModifyAlteration::kExtend:
@@ -66,14 +67,43 @@ BlinkAXEventIntent BlinkAXEventIntent::FromModifiedSelection(
     case TextGranularity::kWord:
       switch (move_direction) {
         case ax::mojom::blink::MoveDirection::kBackward:
+          // All platforms behave the same when moving backward by word.
           text_boundary = ax::mojom::blink::TextBoundary::kWordStart;
           break;
         case ax::mojom::blink::MoveDirection::kForward:
+          switch (platform_word_behavior) {
+            case PlatformWordBehavior::kWordSkipSpaces:
+              // Windows behavior is to always move to the beginning of the next
+              // word.
+              text_boundary = ax::mojom::blink::TextBoundary::kWordStart;
+              break;
+            case PlatformWordBehavior::kWordDontSkipSpaces:
+              // Mac, Linux and ChromeOS behavior is to move to the end of the
+              // current word.
               text_boundary = ax::mojom::blink::TextBoundary::kWordEnd;
               break;
           }
           break;
+      }
+      break;
     case TextGranularity::kSentence:
+      // This granularity always moves to the start of the next or previous
+      // sentence.
+      text_boundary = ax::mojom::blink::TextBoundary::kSentenceStart;
+      break;
+    case TextGranularity::kLine:
+      // This granularity always moves to the start of the next or previous
+      // line.
+      text_boundary = ax::mojom::blink::TextBoundary::kLineStart;
+      break;
+    case TextGranularity::kParagraph:
+      // This granularity always moves to the start of the next or previous
+      // paragraph.
+      text_boundary = ax::mojom::blink::TextBoundary::kParagraphStart;
+      break;
+    case TextGranularity::kSentenceBoundary:
+      // This granularity moves either to the start or the end of the current
+      // sentence, depending on the direction.
       switch (move_direction) {
         case ax::mojom::blink::MoveDirection::kBackward:
           text_boundary = ax::mojom::blink::TextBoundary::kSentenceStart;
@@ -83,7 +113,9 @@ BlinkAXEventIntent BlinkAXEventIntent::FromModifiedSelection(
           break;
       }
       break;
-    case TextGranularity::kLine:
+    case TextGranularity::kLineBoundary:
+      // This granularity moves either to the start or the end of the current
+      // line, depending on the direction.
       switch (move_direction) {
         case ax::mojom::blink::MoveDirection::kBackward:
           text_boundary = ax::mojom::blink::TextBoundary::kLineStart;
@@ -93,7 +125,9 @@ BlinkAXEventIntent BlinkAXEventIntent::FromModifiedSelection(
           break;
       }
       break;
-    case TextGranularity::kParagraph:
+    case TextGranularity::kParagraphBoundary:
+      // This granularity moves either to the start or the end of the current
+      // paragraph, depending on the direction.
       switch (move_direction) {
         case ax::mojom::blink::MoveDirection::kBackward:
           text_boundary = ax::mojom::blink::TextBoundary::kParagraphStart;
@@ -103,15 +137,6 @@ BlinkAXEventIntent BlinkAXEventIntent::FromModifiedSelection(
           break;
       }
       break;
-    case TextGranularity::kSentenceBoundary:
-      text_boundary = ax::mojom::blink::TextBoundary::kSentenceStartOrEnd;
-      break;
-    case TextGranularity::kLineBoundary:
-      text_boundary = ax::mojom::blink::TextBoundary::kLineStartOrEnd;
-      break;
-    case TextGranularity::kParagraphBoundary:
-      text_boundary = ax::mojom::blink::TextBoundary::kParagraphStartOrEnd;
-      break;
     case TextGranularity::kDocumentBoundary:
       text_boundary = ax::mojom::blink::TextBoundary::kWebPage;
       break;

third_party/blink/renderer/core/accessibility/blink_ax_event_intent.h

@@ -11,6 +11,7 @@
 #include "third_party/blink/renderer/core/editing/selection_modifier.h"
 #include "third_party/blink/renderer/core/editing/set_selection_options.h"
 #include "third_party/blink/renderer/core/editing/text_granularity.h"
+#include "third_party/blink/renderer/core/editing/visible_units.h"
 #include "third_party/blink/renderer/platform/text/text_direction.h"
 #include "third_party/blink/renderer/platform/wtf/hash_table_deleted_value_type.h"
 #include "third_party/blink/renderer/platform/wtf/hash_traits.h"
@@ -35,7 +36,8 @@ class CORE_EXPORT BlinkAXEventIntent final {
       const SelectionModifyDirection direction,
       const TextGranularity granularity,
       const SetSelectionBy set_selection_by,
-      const TextDirection direction_of_selection);
+      const TextDirection direction_of_selection,
+      const PlatformWordBehavior platform_word_behavior);
   static BlinkAXEventIntent FromNewSelection(
       const TextGranularity granularity,
       bool is_base_first,

third_party/blink/renderer/core/editing/frame_selection.cc

@@ -425,10 +425,14 @@ bool FrameSelection::Modify(SelectionModifyAlteration alter,
   // Provides details to accessibility about the selection change throughout the
   // current call stack.
   base::AutoReset<bool> is_being_modified_resetter(&is_being_modified_, true);
+  const PlatformWordBehavior platform_word_behavior =
+      frame_->GetEditor().Behavior().ShouldSkipSpaceWhenMovingRight()
+          ? PlatformWordBehavior::kWordSkipSpaces
+          : PlatformWordBehavior::kWordDontSkipSpaces;
   ScopedBlinkAXEventIntent scoped_blink_ax_event_intent(
       BlinkAXEventIntent::FromModifiedSelection(
           alter, direction, granularity, set_selection_by,
-          selection_modifier.DirectionOfSelection()),
+          selection_modifier.DirectionOfSelection(), platform_word_behavior),
       &GetDocument());
 
   // For MacOS only selection is directionless at the beginning.

third_party/blink/web_tests/accessibility/contenteditable-notifications.html

@@ -35,9 +35,9 @@ async_test((t) => {
     // Initial setting of the selection.
     expectedIntents.push('AXEventIntent(setSelection,character,forward)');
     eventSender.keyDown("ArrowDown", []);
-    expectedIntents.push('AXEventIntent(moveSelection,lineEnd,forward)');
+    expectedIntents.push('AXEventIntent(moveSelection,lineStart,forward)');
     eventSender.keyDown("ArrowDown", []);
-    expectedIntents.push('AXEventIntent(moveSelection,lineEnd,forward)');
+    expectedIntents.push('AXEventIntent(moveSelection,lineStart,forward)');
     eventSender.keyDown("ArrowLeft", []);
     expectedIntents.push('AXEventIntent(moveSelection,character,backward)');
     eventSender.keyDown("ArrowLeft", []);
@@ -52,6 +52,6 @@ async_test((t) => {
     eventSender.keyDown("z", []);
     expectedIntents.push('AXEventIntent(setSelection,character,forward)');
   }));
-}, 'This test ensures that moving the cursor in a contentEditable sends a selected text change notification, and typing in a contentEditable sends both a value changed and selected text changed notification.');
+}, 'Moving the cursor in a contentEditable sends a selected text change notification, and typing in a contentEditable sends both a value changed and selected text changed notification.');
 
 </script>

third_party/blink/web_tests/accessibility/contenteditable-selection-with-ignored-nodes.html

@@ -94,7 +94,7 @@ async_test((t) => {
       focusObject: axLine2,
       focusOffset: 0,
       selectedText: '\xA0\n'});  // '\xA0' ==  
-    expectedIntents.push('AXEventIntent(extendSelection,lineEnd,forward)');
+    expectedIntents.push('AXEventIntent(extendSelection,lineStart,forward)');
 
     eventSender.keyDown('ArrowDown', ['shiftKey']);
   });
@@ -106,7 +106,7 @@ async_test((t) => {
       focusObject: axLine3,
       focusOffset: 0,
       selectedText: '\xA0\nApple\n\n'});
-    expectedIntents.push('AXEventIntent(extendSelection,lineEnd,forward)');
+    expectedIntents.push('AXEventIntent(extendSelection,lineStart,forward)');
 
     eventSender.keyDown('ArrowDown', ['shiftKey']);
   });
@@ -118,7 +118,7 @@ async_test((t) => {
       focusObject: axLine3Text,
       focusOffset: 6,
       selectedText: '\xA0\nApple\n\nBanana'});
-    expectedIntents.push('AXEventIntent(extendSelection,lineEnd,forward)');
+    expectedIntents.push('AXEventIntent(extendSelection,lineStart,forward)');
 
     eventSender.keyDown('ArrowDown', ['shiftKey']);
   });