Bug 570710 - avoid extra traversal of children array during tet event creation, r=marcoz, davidb

2024-09-13 09:24:08 -07:00 · 2010-06-10 12:29:56 +09:00 · 2010-06-10 12:29:56 +09:00 · 1f73e9cf4c
commit 1f73e9cf4c
parent 02536d825c
9 changed files with 219 additions and 69 deletions
--- a/accessible/src/base/nsAccEvent.cpp
+++ b/accessible/src/base/nsAccEvent.cpp
@ -394,19 +394,14 @@ NS_IMPL_ISUPPORTS_INHERITED1(nsAccTextChangeEvent, nsAccEvent,
 // XXX revisit this when coalescence is faster (eCoalesceFromSameSubtree)
 nsAccTextChangeEvent::
  nsAccTextChangeEvent(nsIAccessible *aAccessible,
-                       PRInt32 aStart, PRUint32 aLength, PRBool aIsInserted,
+                       PRInt32 aStart, PRUint32 aLength,
+                       nsAString& aModifiedText, PRBool aIsInserted,
                       PRBool aIsAsynch, EIsFromUserInput aIsFromUserInput) :
  nsAccEvent(aIsInserted ? nsIAccessibleEvent::EVENT_TEXT_INSERTED : nsIAccessibleEvent::EVENT_TEXT_REMOVED,
             aAccessible, aIsAsynch, aIsFromUserInput, eAllowDupes),
-  mStart(aStart), mLength(aLength), mIsInserted(aIsInserted)
+  mStart(aStart), mLength(aLength), mIsInserted(aIsInserted),
+  mModifiedText(aModifiedText)
 {
-#ifdef XP_WIN
-  nsCOMPtr<nsIAccessibleText> textAccessible = do_QueryInterface(aAccessible);
-  NS_ASSERTION(textAccessible, "Should not be firing test change event for non-text accessible!!!");
-  if (textAccessible) {
-    textAccessible->GetText(aStart, aStart + aLength, mModifiedText);
-  }
-#endif
 }

 NS_IMETHODIMP
--- a/accessible/src/base/nsAccEvent.h
+++ b/accessible/src/base/nsAccEvent.h
@ -221,7 +221,8 @@ class nsAccTextChangeEvent: public nsAccEvent,
                            public nsIAccessibleTextChangeEvent
 {
 public:
-  nsAccTextChangeEvent(nsIAccessible *aAccessible, PRInt32 aStart, PRUint32 aLength,
+  nsAccTextChangeEvent(nsIAccessible *aAccessible, PRInt32 aStart,
+                       PRUint32 aLength, nsAString& aModifiedText,
                       PRBool aIsInserted, PRBool aIsAsynch = PR_FALSE,
                       EIsFromUserInput aIsFromUserInput = eAutoDetect);

--- a/accessible/src/base/nsAccessible.cpp
+++ b/accessible/src/base/nsAccessible.cpp
@ -2732,6 +2732,24 @@ nsAccessible::GetLinkOffset(PRInt32 *aStartOffset, PRInt32 *aEndOffset)
 nsresult
 nsAccessible::AppendTextTo(nsAString& aText, PRUint32 aStartOffset, PRUint32 aLength)
 {
+  // Return text representation of non-text accessible within hypertext
+  // accessible. Text accessible overrides this method to return enclosed text.
+  if (aStartOffset != 0)
+    return NS_OK;
+
+  nsIFrame *frame = GetFrame();
+  NS_ENSURE_STATE(frame);
+
+  if (frame->GetType() == nsAccessibilityAtoms::brFrame) {
+    aText += kForcedNewLineChar;
+  } else if (nsAccUtils::MustPrune(this)) {
+    // Expose imaginary embedded object character if the accessible hans't
+    // children.
+    aText += kImaginaryEmbeddedObjectChar;
+  } else {
+    aText += kEmbeddedObjectChar;
+  }
+
  return NS_OK;
 }

--- a/accessible/src/base/nsCoreUtils.cpp
+++ b/accessible/src/base/nsCoreUtils.cpp
@ -314,12 +314,13 @@ nsCoreUtils::GetRoleContent(nsIDOMNode *aDOMNode)

 PRBool
 nsCoreUtils::IsAncestorOf(nsINode *aPossibleAncestorNode,
-                          nsINode *aPossibleDescendantNode)
+                          nsINode *aPossibleDescendantNode,
+                          nsINode *aRootNode)
 {
  NS_ENSURE_TRUE(aPossibleAncestorNode && aPossibleDescendantNode, PR_FALSE);

  nsINode *parentNode = aPossibleDescendantNode;
-  while ((parentNode = parentNode->GetNodeParent())) {
+  while ((parentNode = parentNode->GetNodeParent()) != aRootNode) {
    if (parentNode == aPossibleAncestorNode)
      return PR_TRUE;
  }
--- a/accessible/src/base/nsCoreUtils.h
+++ b/accessible/src/base/nsCoreUtils.h
@ -150,11 +150,14 @@ public:
   *                                   aPossibleDescendantNode
   * @param  aPossibleDescendantNode [in] node to test for descendant-ness of
   *                                   aPossibleAncestorNode
+   * @param  aRootNode               [in, optional] the root node that search
+   *                                   search should be performed within
   * @return PR_TRUE                  if aPossibleAncestorNode is an ancestor of
   *                                   aPossibleDescendantNode
   */
   static PRBool IsAncestorOf(nsINode *aPossibleAncestorNode,
-                              nsINode *aPossibleDescendantNode);
+                              nsINode *aPossibleDescendantNode,
+                              nsINode *aRootNode = nsnull);

  /**
   * Helper method to scroll range into view, used for implementation of
--- a/accessible/src/base/nsDocAccessible.cpp
+++ b/accessible/src/base/nsDocAccessible.cpp
@ -1314,55 +1314,58 @@ nsDocAccessible::FireTextChangeEventForText(nsIContent *aContent,
  if (!IsContentLoaded())
    return;

-  nsCOMPtr<nsIDOMNode> node(do_QueryInterface(aContent));
-  if (!node)
+  PRInt32 contentOffset = aInfo->mChangeStart;
+  PRUint32 contentLength = aIsInserted ?
+    aInfo->mReplaceLength: // text has been added
+    aInfo->mChangeEnd - contentOffset; // text has been removed
+
+  if (contentLength == 0)
    return;

-  nsAccessible *accessible = GetAccService()->GetContainerAccessible(node,
-                                                                     PR_TRUE);
+  nsCOMPtr<nsIDOMNode> node(do_QueryInterface(aContent));
+  nsAccessible *accessible = GetAccService()->GetAccessible(node);
  if (!accessible)
    return;

-  nsRefPtr<nsHyperTextAccessible> textAccessible(do_QueryObject(accessible));
+  nsRefPtr<nsHyperTextAccessible> textAccessible =
+    do_QueryObject(accessible->GetParent());
  if (!textAccessible)
    return;

-  PRInt32 start = aInfo->mChangeStart;
-
+  // Get offset within hypertext accessible.
  PRInt32 offset = 0;
-  textAccessible->DOMPointToHypertextOffset(node, start, &offset);
+  textAccessible->DOMPointToHypertextOffset(node, contentOffset, &offset);

-  PRInt32 length = aIsInserted ?
-    aInfo->mReplaceLength: // text has been added
-    aInfo->mChangeEnd - start; // text has been removed
+  nsIFrame* frame = aContent->GetPrimaryFrame();
+  if (!frame)
+    return;

-  if (length > 0) {
-    PRUint32 renderedStartOffset, renderedEndOffset;
-    nsIFrame* frame = aContent->GetPrimaryFrame();
-    if (!frame)
-      return;
+  // Get added or removed text.
+  PRUint32 textOffset = 0;
+  nsresult rv = textAccessible->ContentToRenderedOffset(frame, contentOffset,
+                                                        &textOffset);
+  if (NS_FAILED(rv))
+    return;

-    nsresult rv = textAccessible->ContentToRenderedOffset(frame, start,
-                                                          &renderedStartOffset);
-    if (NS_FAILED(rv))
-      return;
+  nsAutoString text;
+  rv = accessible->AppendTextTo(text, textOffset, contentLength);
+  if (NS_FAILED(rv))
+    return;

-    rv = textAccessible->ContentToRenderedOffset(frame, start + length,
-                                                 &renderedEndOffset);
-    if (NS_FAILED(rv))
-      return;
+  // Get text length.
+  PRUint32 length = text.Length();
+  if (length == 0)
+    return;

-    // Normally we only fire delayed events created from the node, not an
-    // accessible object. See the nsAccTextChangeEvent constructor for details
-    // about this exceptional case.
-    nsRefPtr<nsAccEvent> event =
-      new nsAccTextChangeEvent(accessible, offset,
-                               renderedEndOffset - renderedStartOffset,
+  // Normally we only fire delayed events created from the node, not an
+  // accessible object. See the nsAccTextChangeEvent constructor for details
+  // about this exceptional case.
+  nsRefPtr<nsAccEvent> event =
+      new nsAccTextChangeEvent(textAccessible, offset, length, text,
                               aIsInserted, PR_FALSE);
-    FireDelayedAccessibleEvent(event);
+  FireDelayedAccessibleEvent(event);

-    FireValueChangeForTextFields(accessible);
-  }
+  FireValueChangeForTextFields(textAccessible);
 }

 already_AddRefed<nsAccEvent>
@ -1379,11 +1382,11 @@ nsDocAccessible::CreateTextChangeEventForNode(nsAccessible *aContainerAccessible
    return nsnull;
  }

-  PRInt32 offset;
-  PRInt32 length = 0;
+  PRInt32 offset = 0;
  nsAccessible *changeAcc =
    textAccessible->DOMPointToHypertextOffset(aChangeNode, -1, &offset);

+  nsAutoString text;
  if (!aAccessibleForChangeNode) {
    // A span-level object or something else without an accessible is being removed, where
    // it has no accessible but it has descendant content which is aggregated as text
@ -1396,6 +1399,7 @@ nsDocAccessible::CreateTextChangeEventForNode(nsAccessible *aContainerAccessible
    nsCOMPtr<nsINode> changeNode(do_QueryInterface(aChangeNode));

    nsAccessible *parent = changeAcc->GetParent();
+    nsCOMPtr<nsINode> parentNode = do_QueryInterface(parent->GetDOMNode());
    PRInt32 childCount = parent->GetChildCount();
    PRInt32 changeAccIdx = parent->GetIndexOf(changeAcc);

@ -1403,19 +1407,18 @@ nsDocAccessible::CreateTextChangeEventForNode(nsAccessible *aContainerAccessible
      nsAccessible *child = parent->GetChildAt(idx);
      nsCOMPtr<nsINode> childNode(do_QueryInterface(child->GetDOMNode()));

-      if (!nsCoreUtils::IsAncestorOf(changeNode, childNode)) {
+      if (!nsCoreUtils::IsAncestorOf(changeNode, childNode, parentNode)) {
        // We only want accessibles with DOM nodes as children of this node
        break;
      }

-      length += nsAccUtils::TextLength(child);
+      child->AppendTextTo(text, 0, PR_UINT32_MAX);
    }
  }
  else {
    NS_ASSERTION(!changeAcc || changeAcc == aAccessibleForChangeNode,
                 "Hypertext is reporting a different accessible for this node");

-    length = nsAccUtils::TextLength(aAccessibleForChangeNode);
    if (nsAccUtils::Role(aAccessibleForChangeNode) == nsIAccessibleRole::ROLE_WHITESPACE) {  // newline
      // Don't fire event for the first html:br in an editor.
      nsCOMPtr<nsIEditor> editor;
@ -1428,15 +1431,17 @@ nsDocAccessible::CreateTextChangeEventForNode(nsAccessible *aContainerAccessible
        }
      }
    }
+
+    aAccessibleForChangeNode->AppendTextTo(text, 0, PR_UINT32_MAX);
  }

-  if (length <= 0) {
+  PRUint32 length = text.Length();
+  if (length == 0)
    return nsnull;
-  }

  nsAccEvent *event =
-    new nsAccTextChangeEvent(aContainerAccessible, offset, length, aIsInserting,
-                             aIsAsynch, aIsFromUserInput);
+      new nsAccTextChangeEvent(aContainerAccessible, offset, length, text,
+                               aIsInserting, aIsAsynch, aIsFromUserInput);
  NS_IF_ADDREF(event);

  return event;
--- a/accessible/src/html/nsHyperTextAccessible.cpp
+++ b/accessible/src/html/nsHyperTextAccessible.cpp
@ -431,6 +431,7 @@ nsHyperTextAccessible::GetPosAndText(PRInt32& aStartOffset, PRInt32& aEndOffset,
      else {
        if (endOffset > 0) {
          if (aText) {
+            // XXX: should use nsIAccessible::AppendTextTo.
            if (frame->GetType() == nsAccessibilityAtoms::brFrame) {
              *aText += kForcedNewLineChar;
            } else if (nsAccUtils::MustPrune(this)) {
--- a/accessible/tests/mochitest/events.js
+++ b/accessible/tests/mochitest/events.js
@ -13,6 +13,7 @@ const EVENT_SHOW = nsIAccessibleEvent.EVENT_SHOW;
 const EVENT_STATE_CHANGE = nsIAccessibleEvent.EVENT_STATE_CHANGE;
 const EVENT_TEXT_CARET_MOVED = nsIAccessibleEvent.EVENT_TEXT_CARET_MOVED;
 const EVENT_TEXT_REMOVED = nsIAccessibleEvent.EVENT_TEXT_REMOVED;
+const EVENT_VALUE_CHANGE = nsIAccessibleEvent.EVENT_VALUE_CHANGE;

 ////////////////////////////////////////////////////////////////////////////////
 // General
--- a/accessible/tests/mochitest/events/test_text.html
+++ b/accessible/tests/mochitest/events/test_text.html
@ -19,12 +19,44 @@
          src="chrome://mochikit/content/a11y/accessible/events.js"></script>

  <script type="application/javascript">
+    ////////////////////////////////////////////////////////////////////////////
+    // Invokers
+
    /**
-     * Invokers.
+     * Base text remove invoker and checker.
+     */
+    function textRemoveChecker(aID, aStart, aEnd, aText)
+    {
+      this.target = getNode(aID);
+      this.type = EVENT_TEXT_REMOVED;
+
+      this.check = function textRemoveChecker_check(aEvent)
+      {
+        aEvent.QueryInterface(nsIAccessibleTextChangeEvent);
+        is(aEvent.start, aStart, "Wrong start offset for " + prettyName(aID));
+        is(aEvent.length, aEnd - aStart, "Wrong length for" + prettyName(aID));
+        is(aEvent.isInserted(), false,
+           "Text was removed for " + prettyName(aID));
+        is(aEvent.modifiedText, aText,
+           "Wrong removed text " + prettyName(aID));
+      }
+    }
+
+    function textRemoveInvoker(aID, aStart, aEnd, aText)
+    {
+      this.DOMNode = getNode(aID);
+
+      this.eventSeq = [
+        new textRemoveChecker(aID, aStart, aEnd, aText)
+      ];
+    }
+
+    /**
+     * Remove inaccessible child node containing text accessibles.
     */
    function removeChildSpan(aID)
    {
-      this.DOMNode = getNode(aID);
+      this.__proto__ = new textRemoveInvoker(aID, 0, 5, "33322");

      this.invoke = function removeChildSpan_invoke()
      {
@ -32,25 +64,96 @@
        this.DOMNode.removeChild(this.DOMNode.firstChild);
      }

-      this.eventSeq = [
-        new invokerChecker(EVENT_TEXT_REMOVED, this.DOMNode)
-      ];
-
-      this.check = function removeChildSpan_check(aEvent)
-      {
-        aEvent.QueryInterface(nsIAccessibleTextChangeEvent);
-        is(aEvent.length, 5, "Wrong length of removed text");
-      }
-
-      this.getID = function focusElmWhileSubdocIsFocused_getID()
+      this.getID = function removeChildSpan_getID()
      {
        return "Remove inaccessible span containing accessible nodes" + prettyName(aID);
      }
    }

    /**
-     * Do tests.
+     * Remove child embedded accessible.
     */
+    function removeChildDiv(aID, aChildId)
+    {
+      this.__proto__ = new textRemoveInvoker(aID, 5, 6,
+                                             String.fromCharCode(0xfffc));
+
+      this.invoke = function removeChildDiv_invoke()
+      {
+        var childDiv = this.DOMNode.childNodes[aChildId];
+
+        // Ensure accessible is created to get text remove event when it's
+        // removed.
+        getAccessible(childDiv);
+
+        this.DOMNode.removeChild(childDiv);
+      }
+
+      this.getID = function removeChildDiv_getID()
+      {
+        return "Remove accessible div from the middle of text accessible " +
+          prettyName(aID);
+      }
+    }
+
+    /**
+     * Remove text from HTML input.
+     */
+    function removeTextFromInput(aID, aStart, aEnd, aText)
+    {
+      this.__proto__ = new textRemoveInvoker(aID, aStart, aEnd, aText);
+
+      this.eventSeq.push(new invokerChecker(EVENT_VALUE_CHANGE, this.DOMNode));
+
+      this.invoke = function removeTextFromInput_invoke()
+      {
+        const nsIDOMNSEditableElement =
+          Components.interfaces.nsIDOMNSEditableElement;
+
+        this.DOMNode.focus();
+        this.DOMNode.setSelectionRange(aStart, aEnd);
+
+        synthesizeKey("VK_DELETE", {});
+      }
+
+      this.getID = function removeTextFromInput_getID()
+      {
+        return "Remove text from " + aStart + " to " + aEnd + " for " +
+          prettyName(aID);
+      }
+    }
+
+    /**
+     * Remove text data from text node of editable area.
+     */
+    function removeTextFromEditable(aID, aStart, aEnd, aText, aTextNode)
+    {
+      this.__proto__ = new textRemoveInvoker(aID, aStart, aEnd, aText);
+
+      this.invoke = function removeTextFromEditable_invoke()
+      {
+        this.DOMNode.focus();
+
+        var selection = window.getSelection();
+        var range = document.createRange();
+        range.setStart(this.textNode, aStart);
+        range.setEnd(this.textNode, aEnd);
+        selection.addRange(range);
+
+        synthesizeKey("VK_DELETE", {});
+      }
+
+      this.getID = function removeTextFromEditable_getID()
+      {
+        return "Remove text from " + aStart + " to " + aEnd + " for " +
+          prettyName(aID);
+      }
+
+      this.textNode = getNode(aTextNode);
+    }
+
+    ////////////////////////////////////////////////////////////////////////////
+    // Do tests
    var gQueue = null;
    // gA11yEventDumpID = "eventdump"; // debug stuff

@ -62,6 +165,19 @@
      // accessible text nodes.
      gQueue.push(new removeChildSpan("p"));

+      // Remove embedded character.
+      gQueue.push(new removeChildDiv("div", 1));
+
+      // Text remove from text node within hypertext accessible.
+      gQueue.push(new removeTextFromInput("input", 1, 3, "al"));
+
+      // bug 570691
+      todo(false, "Fix text change events from editable area, see bug 570691");
+      //var textNode = getNode("editable").firstChild;
+      //gQueue.push(new removeTextFromEditable("editable", 1, 3, "al", textNode));
+      //textNode = getNode("editable2").firstChild.firstChild;
+      //gQueue.push(new removeTextFromEditable("editable2", 1, 3, "al", textNode));
+
      gQueue.invoke(); // Will call SimpleTest.finish();
    }

@ -77,6 +193,11 @@
     title=" wrong length of text remove event when inaccessible node containing accessible nodes is removed">
    Mozilla Bug 566293
  </a>
+  <a target="_blank"
+     href="https://bugzilla.mozilla.org/show_bug.cgi?id=570710"
+     title="Avoid extra array traversal during text event creation">
+    Mozilla Bug 570710
+  </a>

  <p id="display"></p>
  <div id="content" style="display: none"></div>
@ -85,5 +206,9 @@
  <div id="eventdump"></div>

  <p id="p"><span><span>333</span><span>22</span></span>1111</p>
+  <div id="div">hello<div>hello</div>hello</div>
+  <input id="input" value="value">
+  <div contentEditable="true" id="editable">value</div>
+  <div contentEditable="true" id="editable2"><span>value</span></div>
 </body>
 </html>