Bug 1653039 - Add word at marker APIs. r=morgan
authorEitan Isaacson <eitan@monotonous.org>
Mon, 20 Jul 2020 22:53:04 +0000
changeset 541355 8dc0120d47e63446d78b76887d53587031621d76
parent 541354 390e9e8df3c849d0be93a1646672146b021d0ae6
child 541356 743486fcc01e3554cd06bfdc6de75a8d14d60736
push id37621
push userrmaries@mozilla.com
push dateTue, 21 Jul 2020 09:42:41 +0000
treeherdermozilla-central@501791d4676e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmorgan
bugs1653039
milestone80.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1653039 - Add word at marker APIs. r=morgan This patch adds several new parameters: * AXLeftWordTextMarkerRangeForTextMarker * AXRightWordTextMarkerRangeForTextMarker * AXStartTextMarkerForTextMarkerRange * AXEndTextMarkerForTextMarkerRange * AXNextTextMarkerForTextMarker * AXPreviousTextMarkerForTextMarker Our word boundaries API is pretty buggy. Instead of trying to resolve or triage each issue I found, I added todo tests for them. Differential Revision: https://phabricator.services.mozilla.com/D83680
accessible/mac/GeckoTextMarker.h
accessible/mac/GeckoTextMarker.mm
accessible/mac/MOXAccessibleProtocol.h
accessible/mac/MOXTextMarkerDelegate.h
accessible/mac/MOXTextMarkerDelegate.mm
accessible/tests/browser/mac/browser_text_basics.js
--- a/accessible/mac/GeckoTextMarker.h
+++ b/accessible/mac/GeckoTextMarker.h
@@ -11,35 +11,56 @@
 
 typedef CFTypeRef AXTextMarkerRef;
 typedef CFTypeRef AXTextMarkerRangeRef;
 
 namespace mozilla {
 namespace a11y {
 
 class AccessibleOrProxy;
+class GeckoTextMarkerRange;
 
 class GeckoTextMarker final {
  public:
   GeckoTextMarker(const AccessibleOrProxy& aContainer, int32_t aOffset)
       : mContainer(aContainer), mOffset(aOffset) {}
 
   GeckoTextMarker(const GeckoTextMarker& aPoint)
       : mContainer(aPoint.mContainer), mOffset(aPoint.mOffset) {}
 
   GeckoTextMarker(AccessibleOrProxy aDoc, AXTextMarkerRef aTextMarker);
 
   GeckoTextMarker() : mContainer(nullptr), mOffset(0) {}
 
   id CreateAXTextMarker();
 
+  // Mutate marker so that its offset references an actual character
+  // and not an embedded link. Or, if the offset is at the end of the
+  // container, mutate the marker to the end offset of an ancestor
+  // container that has following non-link text.
+  void NormalizeNext();
+
+  // Mutate the marker so that its offset is preceded by a non-link
+  // offset, If the marker's offset is at the begining of a container,
+  // mutate the marker to point to the top-most link offset in an ancestor.
+  void NormalizePrevious();
+
+  // Return true if offset is at the end of the container.
+  bool AtEnd() { return static_cast<uint32_t>(mOffset) >= CharacterCount(mContainer); }
+
+  // Return a word range for the given offset.
+  GeckoTextMarkerRange WordRange();
+
   bool operator<(const GeckoTextMarker& aPoint) const;
 
   AccessibleOrProxy mContainer;
   int32_t mOffset;
+
+ private:
+  uint32_t CharacterCount(const AccessibleOrProxy& aContainer);
 };
 
 class GeckoTextMarkerRange final {
  public:
   GeckoTextMarkerRange(const GeckoTextMarker& aStart, const GeckoTextMarker& aEnd)
       : mStart(aStart), mEnd(aEnd) {}
 
   GeckoTextMarkerRange(AccessibleOrProxy aDoc, AXTextMarkerRangeRef aTextMarkerRange);
--- a/accessible/mac/GeckoTextMarker.mm
+++ b/accessible/mac/GeckoTextMarker.mm
@@ -95,16 +95,124 @@ bool GeckoTextMarker::operator<(const Ge
       return child1.IndexInParent() < child2.IndexInParent();
     }
   }
 
   MOZ_ASSERT_UNREACHABLE("Broken tree?!");
   return false;
 }
 
+void GeckoTextMarker::NormalizeNext() {
+  if (AtEnd()) {
+    // If this is the end of the current container, mutate to its parent's
+    // end offset.
+    bool unused;
+    uint32_t endOffset = mContainer.IsProxy() ? mContainer.AsProxy()->EndOffset(&unused)
+                                              : mContainer.AsAccessible()->EndOffset();
+
+    if (endOffset != 0) {
+      mContainer = mContainer.Parent();
+      mOffset = endOffset;
+
+      // Call NormalizeNext recursively to get top-most link if at the end of one,
+      // or innermost link if at the beginning.
+      NormalizeNext();
+    }
+  } else {
+    AccessibleOrProxy link;
+
+    if (mContainer.IsProxy()) {
+      ProxyAccessible* proxy = mContainer.AsProxy();
+      link = proxy->LinkAt(proxy->LinkIndexAtOffset(mOffset));
+    } else if (HyperTextAccessible* ht = mContainer.AsAccessible()->AsHyperText()) {
+      link = ht->LinkAt(ht->LinkIndexAtOffset(mOffset));
+    }
+
+    // If there is a link at this offset, mutate into it.
+    if (!link.IsNull()) {
+      mContainer = link;
+      mOffset = 0;
+
+      // Call NormalizeNext recursively to get top-most link if at the end of one,
+      // or innermost link if at the beginning.
+      NormalizeNext();
+    }
+  }
+}
+
+void GeckoTextMarker::NormalizePrevious() {
+  if (mOffset == 0) {
+    // If we are at the beginning of a container, mutate to its parent's start offset.
+    bool unused;
+    uint32_t startOffset = mContainer.IsProxy() ? mContainer.AsProxy()->StartOffset(&unused)
+                                                : mContainer.AsAccessible()->StartOffset();
+
+    if (startOffset != 0) {
+      mContainer = mContainer.Parent();
+      mOffset = startOffset;
+
+      // Call NormalizePrevious recursively to get top-most link if at the start of one,
+      // or innermost link if at the end.
+      NormalizePrevious();
+    }
+  } else {
+    AccessibleOrProxy link;
+
+    if (mContainer.IsProxy()) {
+      ProxyAccessible* proxy = mContainer.AsProxy();
+      link = proxy->LinkAt(proxy->LinkIndexAtOffset(mOffset - 1));
+    } else if (HyperTextAccessible* ht = mContainer.AsAccessible()->AsHyperText()) {
+      link = ht->GetChildAtOffset(mOffset - 1);
+    }
+
+    // If there is a link preceding this offset, mutate into it.
+    if (!link.IsNull()) {
+      mContainer = link;
+      mOffset = CharacterCount(link);
+
+      // Call NormalizePrevious recursively to get top-most link if at the start of one,
+      // or innermost link if at the end.
+      NormalizePrevious();
+    }
+  }
+}
+
+uint32_t GeckoTextMarker::CharacterCount(const AccessibleOrProxy& aContainer) {
+  if (aContainer.IsProxy()) {
+    return aContainer.AsProxy()->CharacterCount();
+  }
+
+  if (aContainer.AsAccessible()->IsHyperText()) {
+    return aContainer.AsAccessible()->AsHyperText()->CharacterCount();
+  }
+
+  return 0;
+}
+
+GeckoTextMarkerRange GeckoTextMarker::WordRange() {
+  int32_t wordstart_start = 0, wordstart_end = 0, wordend_start = 0, wordend_end = 0;
+  nsAutoString unused;
+  if (mContainer.IsProxy()) {
+    mContainer.AsProxy()->GetTextAtOffset(mOffset, nsIAccessibleText::BOUNDARY_WORD_START, unused,
+                                          &wordstart_start, &wordstart_end);
+    mContainer.AsProxy()->GetTextAtOffset(mOffset, nsIAccessibleText::BOUNDARY_WORD_END, unused,
+                                          &wordend_start, &wordend_end);
+  } else if (HyperTextAccessible* ht = mContainer.AsAccessible()->AsHyperText()) {
+    ht->TextAtOffset(mOffset, nsIAccessibleText::BOUNDARY_WORD_START, &wordstart_start,
+                     &wordstart_end, unused);
+    ht->TextAtOffset(mOffset, nsIAccessibleText::BOUNDARY_WORD_END, &wordend_start, &wordend_end,
+                     unused);
+  }
+
+  // We use the intersecting boundary of word start, and word end because we don't have
+  // a boundary mode that is non-inclusive of whitespace.
+  return GeckoTextMarkerRange(GeckoTextMarker(mContainer, std::max(wordstart_start, wordend_start)),
+                              GeckoTextMarker(mContainer, std::min(wordstart_end, wordend_end)));
+}
+
 // GeckoTextMarkerRange
 
 GeckoTextMarkerRange::GeckoTextMarkerRange(AccessibleOrProxy aDoc,
                                            AXTextMarkerRangeRef aTextMarkerRange) {
   if (CFGetTypeID(aTextMarkerRange) != AXTextMarkerRangeGetTypeID()) {
     return;
   }
 
--- a/accessible/mac/MOXAccessibleProtocol.h
+++ b/accessible/mac/MOXAccessibleProtocol.h
@@ -324,9 +324,27 @@
 - (NSNumber* _Nullable)moxLengthForTextMarkerRange:(id _Nonnull)textMarkerRange;
 
 // AXStringForTextMarkerRange
 - (NSString* _Nullable)moxStringForTextMarkerRange:(id _Nonnull)textMarkerRange;
 
 // AXTextMarkerRangeForUnorderedTextMarkers
 - (id _Nullable)moxTextMarkerRangeForUnorderedTextMarkers:(NSArray* _Nonnull)textMarkers;
 
+// AXLeftWordTextMarkerRangeForTextMarker
+- (id _Nullable)moxLeftWordTextMarkerRangeForTextMarker:(id _Nonnull)textMarker;
+
+// AXRightWordTextMarkerRangeForTextMarker
+- (id _Nullable)moxRightWordTextMarkerRangeForTextMarker:(id _Nonnull)textMarker;
+
+// AXStartTextMarkerForTextMarkerRange
+- (id _Nullable)moxStartTextMarkerForTextMarkerRange:(id _Nonnull)textMarkerRange;
+
+// AXEndTextMarkerForTextMarkerRange
+- (id _Nullable)moxEndTextMarkerForTextMarkerRange:(id _Nonnull)textMarkerRange;
+
+// AXNextTextMarkerForTextMarker
+- (id _Nullable)moxNextTextMarkerForTextMarker:(id _Nonnull)textMarker;
+
+// AXPreviousTextMarkerForTextMarker
+- (id _Nullable)moxPreviousTextMarkerForTextMarker:(id _Nonnull)textMarker;
+
 @end
--- a/accessible/mac/MOXTextMarkerDelegate.h
+++ b/accessible/mac/MOXTextMarkerDelegate.h
@@ -42,9 +42,27 @@
 - (NSNumber*)moxLengthForTextMarkerRange:(id)textMarkerRange;
 
 // override
 - (NSString*)moxStringForTextMarkerRange:(id)textMarkerRange;
 
 // override
 - (id)moxTextMarkerRangeForUnorderedTextMarkers:(NSArray*)textMarkers;
 
+// override
+- (id)moxStartTextMarkerForTextMarkerRange:(id)textMarkerRange;
+
+// override
+- (id)moxEndTextMarkerForTextMarkerRange:(id)textMarkerRange;
+
+// override
+- (id)moxLeftWordTextMarkerRangeForTextMarker:(id)textMarker;
+
+// override
+- (id)moxRightWordTextMarkerRangeForTextMarker:(id)textMarker;
+
+// override
+- (id)moxNextTextMarkerForTextMarker:(id)textMarker;
+
+// override
+- (id)moxPreviousTextMarkerForTextMarker:(id)textMarker;
+
 @end
--- a/accessible/mac/MOXTextMarkerDelegate.mm
+++ b/accessible/mac/MOXTextMarkerDelegate.mm
@@ -124,9 +124,74 @@ static nsDataHashtable<nsUint64HashKey, 
   GeckoTextMarker p2(mGeckoDocAccessible, textMarkers[1]);
 
   bool ordered = p1 < p2;
   GeckoTextMarkerRange range(ordered ? p1 : p2, ordered ? p2 : p1);
 
   return range.CreateAXTextMarkerRange();
 }
 
+- (id)moxStartTextMarkerForTextMarkerRange:(id)textMarkerRange {
+  mozilla::a11y::GeckoTextMarkerRange range(mGeckoDocAccessible, textMarkerRange);
+
+  return range.IsValid() ? range.mStart.CreateAXTextMarker() : nil;
+}
+
+- (id)moxEndTextMarkerForTextMarkerRange:(id)textMarkerRange {
+  mozilla::a11y::GeckoTextMarkerRange range(mGeckoDocAccessible, textMarkerRange);
+
+  return range.IsValid() ? range.mEnd.CreateAXTextMarker() : nil;
+}
+
+- (id)moxLeftWordTextMarkerRangeForTextMarker:(id)textMarker {
+  GeckoTextMarker geckoTextMarker(mGeckoDocAccessible, textMarker);
+  geckoTextMarker.NormalizePrevious();
+
+  if (geckoTextMarker.mOffset == 0) {
+    // We are probably at the start of the root container, normalize next
+    // so we get the first word.
+    geckoTextMarker.NormalizeNext();
+  } else {
+    // Go to preceding offset to get "left" word.
+    geckoTextMarker.mOffset--;
+    geckoTextMarker.NormalizePrevious();
+  }
+
+  return geckoTextMarker.WordRange().CreateAXTextMarkerRange();
+}
+
+- (id)moxRightWordTextMarkerRangeForTextMarker:(id)textMarker {
+  GeckoTextMarker geckoTextMarker(mGeckoDocAccessible, textMarker);
+  geckoTextMarker.NormalizeNext();
+
+  GeckoTextMarkerRange range = geckoTextMarker.AtEnd()
+                                   ? GeckoTextMarkerRange(geckoTextMarker, geckoTextMarker)
+                                   : geckoTextMarker.WordRange();
+
+  return range.CreateAXTextMarkerRange();
+}
+
+- (id)moxNextTextMarkerForTextMarker:(id)textMarker {
+  GeckoTextMarker geckoTextMarker(mGeckoDocAccessible, textMarker);
+
+  geckoTextMarker.NormalizeNext();
+  if (geckoTextMarker.AtEnd()) {
+    return nil;
+  }
+
+  geckoTextMarker.mOffset++;
+
+  return geckoTextMarker.CreateAXTextMarker();
+}
+
+- (id)moxPreviousTextMarkerForTextMarker:(id)textMarker {
+  GeckoTextMarker geckoTextMarker(mGeckoDocAccessible, textMarker);
+
+  geckoTextMarker.NormalizePrevious();
+  if (geckoTextMarker.mOffset == 0) {
+    return nil;
+  }
+
+  geckoTextMarker.mOffset--;
+  return geckoTextMarker.CreateAXTextMarker();
+}
+
 @end
--- a/accessible/tests/browser/mac/browser_text_basics.js
+++ b/accessible/tests/browser/mac/browser_text_basics.js
@@ -6,44 +6,216 @@
 
 /* import-globals-from ../../mochitest/role.js */
 /* import-globals-from ../../mochitest/states.js */
 loadScripts(
   { name: "role.js", dir: MOCHITESTS_DIR },
   { name: "states.js", dir: MOCHITESTS_DIR }
 );
 
+function stringForRange(macDoc, range) {
+  return macDoc.getParameterizedAttributeValue(
+    "AXStringForTextMarkerRange",
+    range
+  );
+}
+
+function testWordAtMarker(
+  macDoc,
+  marker,
+  expectedLeft,
+  expectedRight,
+  options = {}
+) {
+  let left = macDoc.getParameterizedAttributeValue(
+    "AXLeftWordTextMarkerRangeForTextMarker",
+    marker
+  );
+  let right = macDoc.getParameterizedAttributeValue(
+    "AXRightWordTextMarkerRangeForTextMarker",
+    marker
+  );
+  (options.leftIs || is)(
+    stringForRange(macDoc, left),
+    expectedLeft,
+    "Left word matches"
+  );
+  (options.rightIs || is)(
+    stringForRange(macDoc, right),
+    expectedRight,
+    "Right word matches"
+  );
+
+  return macDoc.getParameterizedAttributeValue(
+    "AXNextTextMarkerForTextMarker",
+    marker
+  );
+}
+
 // Read-only tests
 addAccessibleTask(`<p id="p">Hello World</p>`, async (browser, accDoc) => {
   let macDoc = accDoc.nativeInterface.QueryInterface(
     Ci.nsIAccessibleMacInterface
   );
 
-  function stringForRange(r) {
-    return macDoc.getParameterizedAttributeValue(
-      "AXStringForTextMarkerRange",
-      r
-    );
-  }
-
   let startMarker = macDoc.getAttributeValue("AXStartTextMarker");
   let endMarker = macDoc.getAttributeValue("AXEndTextMarker");
   let range = macDoc.getParameterizedAttributeValue(
     "AXTextMarkerRangeForUnorderedTextMarkers",
     [startMarker, endMarker]
   );
-  is(stringForRange(range), "Hello World");
+  is(stringForRange(macDoc, range), "Hello World");
 
   let evt = waitForMacEvent("AXSelectedTextChanged");
   await SpecialPowers.spawn(browser, [], () => {
     let p = content.document.getElementById("p");
     let r = new content.Range();
     r.setStart(p.firstChild, 1);
     r.setEnd(p.firstChild, 8);
 
     let s = content.getSelection();
     s.addRange(r);
   });
   await evt;
 
   range = macDoc.getAttributeValue("AXSelectedTextMarkerRange");
-  is(stringForRange(range), "ello Wo");
+  is(stringForRange(macDoc, range), "ello Wo");
+});
+
+addAccessibleTask(`<p>hello world goodbye</p>`, async (browser, accDoc) => {
+  let macDoc = accDoc.nativeInterface.QueryInterface(
+    Ci.nsIAccessibleMacInterface
+  );
+
+  let marker = macDoc.getAttributeValue("AXStartTextMarker");
+
+  function testWordAndAdvance(left, right) {
+    testWordAtMarker(macDoc, marker, left, right);
+    marker = macDoc.getParameterizedAttributeValue(
+      "AXNextTextMarkerForTextMarker",
+      marker
+    );
+  }
+
+  testWordAndAdvance("hello", "hello");
+  testWordAndAdvance("hello", "hello");
+  testWordAndAdvance("hello", "hello");
+  testWordAndAdvance("hello", "hello");
+  testWordAndAdvance("hello", "hello");
+  testWordAndAdvance("hello", " ");
+  testWordAndAdvance(" ", "world");
+  testWordAndAdvance("world", "world");
+  testWordAndAdvance("world", "world");
+  testWordAndAdvance("world", "world");
+  testWordAndAdvance("world", "world");
+  testWordAndAdvance("world", " ");
+  testWordAndAdvance(" ", "goodbye");
+  testWordAndAdvance("goodbye", "goodbye");
+  testWordAndAdvance("goodbye", "goodbye");
+  testWordAndAdvance("goodbye", "goodbye");
+  testWordAndAdvance("goodbye", "goodbye");
+  testWordAndAdvance("goodbye", "goodbye");
+  testWordAndAdvance("goodbye", "goodbye");
+  testWordAndAdvance("goodbye", "");
+
+  ok(!marker, "Iterated through all markers");
 });
+
+addAccessibleTask(
+  `<p>hello world <a href="#">i love you</a> goodbye</p>`,
+  async (browser, accDoc) => {
+    let macDoc = accDoc.nativeInterface.QueryInterface(
+      Ci.nsIAccessibleMacInterface
+    );
+
+    let marker = macDoc.getAttributeValue("AXStartTextMarker");
+
+    function testWordAndAdvance(left, right, options = {}) {
+      testWordAtMarker(macDoc, marker, left, right, options);
+      marker = macDoc.getParameterizedAttributeValue(
+        "AXNextTextMarkerForTextMarker",
+        marker
+      );
+    }
+
+    testWordAndAdvance("hello", "hello");
+    testWordAndAdvance("hello", "hello");
+    testWordAndAdvance("hello", "hello");
+    testWordAndAdvance("hello", "hello");
+    testWordAndAdvance("hello", "hello");
+    testWordAndAdvance("hello", " ");
+    testWordAndAdvance(" ", "world");
+    testWordAndAdvance("world", "world");
+    testWordAndAdvance("world", "world");
+    testWordAndAdvance("world", "world");
+    testWordAndAdvance("world", "world");
+    // Expected " ", got ""
+    testWordAndAdvance("world", " ", { rightIs: todo_is });
+    // Expected " ", got ""
+    testWordAndAdvance(" ", "i", { leftIs: todo_is });
+    // Expected "i", got "i love you"
+    testWordAndAdvance("i", " ", { leftIs: todo_is });
+    testWordAndAdvance(" ", "love");
+    testWordAndAdvance("love", "love");
+    testWordAndAdvance("love", "love");
+    testWordAndAdvance("love", "love");
+    testWordAndAdvance("love", " ");
+    testWordAndAdvance(" ", "you");
+    testWordAndAdvance("you", "you");
+    testWordAndAdvance("you", "you");
+    // Expected " ", got "i love you"
+    testWordAndAdvance("you", " ", { rightIs: todo_is });
+    // Expected " ", for "you"
+    testWordAndAdvance(" ", "goodbye", { leftIs: todo_is });
+    testWordAndAdvance("goodbye", "goodbye");
+    testWordAndAdvance("goodbye", "goodbye");
+    testWordAndAdvance("goodbye", "goodbye");
+    testWordAndAdvance("goodbye", "goodbye");
+    testWordAndAdvance("goodbye", "goodbye");
+    testWordAndAdvance("goodbye", "goodbye");
+    testWordAndAdvance("goodbye", "", { expectNoNextMarker: true });
+  }
+);
+
+addAccessibleTask(
+  `<p>hello <a href=#">wor</a>ld goodbye</p>`,
+  async (browser, accDoc) => {
+    let macDoc = accDoc.nativeInterface.QueryInterface(
+      Ci.nsIAccessibleMacInterface
+    );
+
+    let marker = macDoc.getAttributeValue("AXStartTextMarker");
+
+    for (let i = 0; i < 7; i++) {
+      marker = macDoc.getParameterizedAttributeValue(
+        "AXNextTextMarkerForTextMarker",
+        marker
+      );
+    }
+
+    let left = macDoc.getParameterizedAttributeValue(
+      "AXLeftWordTextMarkerRangeForTextMarker",
+      marker
+    );
+    let right = macDoc.getParameterizedAttributeValue(
+      "AXRightWordTextMarkerRangeForTextMarker",
+      marker
+    );
+    is(stringForRange(macDoc, left), "world", "Left word matches");
+    todo_is(stringForRange(macDoc, right), "world", "Right word matches");
+
+    marker = macDoc.getParameterizedAttributeValue(
+      "AXNextTextMarkerForTextMarker",
+      marker
+    );
+
+    left = macDoc.getParameterizedAttributeValue(
+      "AXLeftWordTextMarkerRangeForTextMarker",
+      marker
+    );
+    right = macDoc.getParameterizedAttributeValue(
+      "AXRightWordTextMarkerRangeForTextMarker",
+      marker
+    );
+    todo_is(stringForRange(macDoc, left), "world", "Left word matches");
+    todo_is(stringForRange(macDoc, right), "world", "Right word matches");
+  }
+);