Bug 1250333 - do not create accessibles for trailing BRs, r=davidb, roc
authorAlexander Surkov <surkov.alexander@gmail.com>
Thu, 25 Feb 2016 07:09:59 -0500
changeset 285558 2f43bc2f7ffdec7157f47793397110f9e8651cdf
parent 285557 3b913f81cb98b75061bb8c90f7780f88ac4b7dbb
child 285559 bf293d7d7ac6177bd73bddefce083847607f5bb7
push id30031
push userkwierso@gmail.com
push dateThu, 25 Feb 2016 22:25:25 +0000
treeherdermozilla-central@97cf677ee668 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidb, roc
bugs1250333
milestone47.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 1250333 - do not create accessibles for trailing BRs, r=davidb, roc
accessible/generic/DocAccessible.cpp
accessible/generic/HyperTextAccessible.cpp
accessible/generic/HyperTextAccessible.h
accessible/tests/mochitest/common.js
accessible/tests/mochitest/editabletext/editabletext.js
layout/generic/nsBRFrame.cpp
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -1425,31 +1425,20 @@ void
 DocAccessible::CacheChildren()
 {
   // Search for accessible children starting from the document element since
   // some web pages tend to insert elements under it rather than document body.
   dom::Element* rootElm = mDocumentNode->GetRootElement();
   if (!rootElm)
     return;
 
-  // Ignore last HTML:br, copied from HyperTextAccessible.
   TreeWalker walker(this, rootElm);
-  Accessible* lastChild = nullptr;
-  while (Accessible* child = walker.Next()) {
-    if (lastChild)
-      AppendChild(lastChild);
-
-    lastChild = child;
-  }
-
-  if (lastChild) {
-    if (lastChild->IsHTMLBr())
-      Document()->UnbindFromDocument(lastChild);
-    else
-      AppendChild(lastChild);
+  Accessible* child = nullptr;
+  while ((child = walker.Next())) {
+    AppendChild(child);
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Protected members
 
 void
 DocAccessible::NotifyOfLoading(bool aIsReloading)
--- a/accessible/generic/HyperTextAccessible.cpp
+++ b/accessible/generic/HyperTextAccessible.cpp
@@ -1930,41 +1930,16 @@ HyperTextAccessible::RelationByType(Rela
       break;
     default:
       break;
   }
 
   return rel;
 }
 
-void
-HyperTextAccessible::CacheChildren()
-{
-  // Trailing HTML br element don't play any difference. We don't need to expose
-  // it to AT (see bug https://bugzilla.mozilla.org/show_bug.cgi?id=899433#c16
-  // for details).
-
-  TreeWalker walker(this, mContent);
-  Accessible* child = nullptr;
-  Accessible* lastChild = nullptr;
-  while ((child = walker.Next())) {
-    if (lastChild)
-      AppendChild(lastChild);
-
-    lastChild = child;
-  }
-
-  if (lastChild) {
-    if (lastChild->IsHTMLBr())
-      Document()->UnbindFromDocument(lastChild);
-    else
-      AppendChild(lastChild);
-  }
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // HyperTextAccessible public static
 
 nsresult
 HyperTextAccessible::ContentToRenderedOffset(nsIFrame* aFrame, int32_t aContentOffset,
                                              uint32_t* aRenderedOffset) const
 {
   if (!aFrame) {
--- a/accessible/generic/HyperTextAccessible.h
+++ b/accessible/generic/HyperTextAccessible.h
@@ -429,17 +429,16 @@ public:
    */
   dom::Selection* DOMSelection() const;
 
 protected:
   virtual ~HyperTextAccessible() { }
 
   // Accessible
   virtual ENameValueFlag NativeName(nsString& aName) override;
-  virtual void CacheChildren() override;
 
   // HyperTextAccessible
 
   /**
    * Transform magic offset into text offset.
    */
   index_t ConvertMagicOffset(int32_t aOffset) const;
 
--- a/accessible/tests/mochitest/common.js
+++ b/accessible/tests/mochitest/common.js
@@ -121,19 +121,33 @@ function dumpTree(aId, aMsg)
 
     var children = acc.children;
     for (var i = 0; i < children.length; i++) {
       var child = children.queryElementAt(i, nsIAccessible);
       dumpTreeIntl(child, indent + "  ");
     }
   }
 
+  function dumpDOMTreeIntl(node, indent)
+  {
+    dump(indent + prettyName(node) + "\n");
+
+    var children = node.childNodes;
+    for (var i = 0; i < children.length; i++) {
+      var child = children.item(i);
+      dumpDOMTreeIntl(child, indent + "  ");
+    }
+  }
+
   dump(aMsg + "\n");
   var root = getAccessible(aId);
   dumpTreeIntl(root, "  ");
+
+  dump("DOM tree:\n");
+  dumpDOMTreeIntl(getNode(aId), "  ");
 }
 
 /**
  * Invokes the given function when document is loaded and focused. Preferable
  * to mochitests 'addLoadEvent' function -- additionally ensures state of the
  * document accessible is not busy.
  *
  * @param aFunc  the function to invoke
--- a/accessible/tests/mochitest/editabletext/editabletext.js
+++ b/accessible/tests/mochitest/editabletext/editabletext.js
@@ -57,16 +57,17 @@ function editableTextTest(aID)
    * setTextContents test.
    */
   this.setTextContents = function setTextContents(aValue, aSkipStartOffset)
   {
     var testID = "setTextContents '" + aValue + "' for " + prettyName(aID);
 
     function setTextContentsInvoke()
     {
+      dump(`\nsetTextContents '${aValue}'\n`);
       var acc = getAccessible(aID, nsIAccessibleEditableText);
       acc.setTextContents(aValue);
     }
 
     aSkipStartOffset = aSkipStartOffset || 0;
     var insertTripple = aValue ?
       [ aSkipStartOffset, aSkipStartOffset + aValue.length, aValue ] : null;
     var oldValue = getValue(aID);
@@ -82,16 +83,17 @@ function editableTextTest(aID)
    */
   this.insertText = function insertText(aStr, aPos, aResStr, aResPos)
   {
     var testID = "insertText '" + aStr + "' at " + aPos + " for " +
       prettyName(aID);
 
     function insertTextInvoke()
     {
+      dump(`\ninsertText '${aStr}' at ${aPos} pos\n`);
       var acc = getAccessible(aID, nsIAccessibleEditableText);
       acc.insertText(aStr, aPos);
     }
 
     var resPos = (aResPos != undefined) ? aResPos : aPos;
     this.generateTest(aID, null, [resPos, resPos + aStr.length, aStr],
                       insertTextInvoke, getValueChecker(aID, aResStr), testID);
   }
--- a/layout/generic/nsBRFrame.cpp
+++ b/layout/generic/nsBRFrame.cpp
@@ -262,12 +262,19 @@ BRFrame::AccessibleType()
   nsIContent *parent = mContent->GetParent();
   if (parent && parent->IsRootOfNativeAnonymousSubtree() &&
       parent->GetChildCount() == 1) {
     // This <br> is the only node in a text control, therefore it is the hacky
     // "bogus node" used when there is no text in the control
     return a11y::eNoType;
   }
 
+  // Trailing HTML br element don't play any difference. We don't need to expose
+  // it to AT (see bug https://bugzilla.mozilla.org/show_bug.cgi?id=899433#c16
+  // for details).
+  if (!mContent->GetNextSibling() && !GetNextSibling()) {
+    return a11y::eNoType;
+  }
+
   return a11y::eHTMLBRType;
 }
 #endif