Bug 899433 - Accessibility returns empty line for last line in certain cases, r=tbsaunde

accessible/src/generic/Accessible.h

@@ -481,6 +481,7 @@ public:
   bool IsHyperText() const { return HasGenericType(eHyperText); }
   HyperTextAccessible* AsHyperText();
 
+  bool IsHTMLBr() const { return mType == eHTMLBRType; }
   bool IsHTMLFileInput() const { return mType == eHTMLFileInputType; }
 
   bool IsHTMLListItem() const { return mType == eHTMLLiType; }

accessible/src/generic/HyperTextAccessible.cpp

@@ -13,6 +13,7 @@
 #include "Role.h"
 #include "States.h"
 #include "TextAttrs.h"
+#include "TreeWalker.h"
 
 #include "nsIClipboard.h"
 #include "nsContentUtils.h"
@@ -2062,6 +2063,31 @@ HyperTextAccessible::RemoveChild(Accessible* aAccessible)
   return Accessible::RemoveChild(aAccessible);
 }
 
+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.NextChild())) {
+    if (lastChild)
+      AppendChild(lastChild);
+
+    lastChild = child;
+  }
+
+  if (lastChild) {
+    if (lastChild->IsHTMLBr())
+      Document()->UnbindFromDocument(lastChild);
+    else
+      AppendChild(lastChild);
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // HyperTextAccessible public static
 

accessible/src/generic/HyperTextAccessible.h

@@ -273,6 +273,7 @@ public:
 protected:
   // Accessible
   virtual ENameValueFlag NativeName(nsString& aName) MOZ_OVERRIDE;
+  virtual void CacheChildren() MOZ_OVERRIDE;
 
   // HyperTextAccessible
 

accessible/src/html/HTMLElementAccessibles.h

@@ -33,7 +33,10 @@ class HTMLBRAccessible : public LeafAccessible
 {
 public:
   HTMLBRAccessible(nsIContent* aContent, DocAccessible* aDoc) :
-    LeafAccessible(aContent, aDoc) {}
+    LeafAccessible(aContent, aDoc)
+  {
+    mType = eHTMLBRType;
+  }
 
   // Accessible
   virtual a11y::role NativeRole();

accessible/src/html/HTMLFormControlAccessible.cpp

@@ -464,22 +464,6 @@ HTMLTextFieldAccessible::GetEditor() const
   return editor.forget();
 }
 
-void
-HTMLTextFieldAccessible::CacheChildren()
-{
-  // XXX: textarea shouldn't contain anything but text leafs. Currently it may
-  // contain a trailing fake HTML br element added for layout needs. We don't
-  // need to expose it since it'd be confusing for AT.
-  TreeWalker walker(this, mContent);
-  Accessible* child = nullptr;
-  while ((child = walker.NextChild())) {
-    if (child->IsTextLeaf())
-      AppendChild(child);
-    else
-      Document()->UnbindFromDocument(child);
-  }
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // HTMLTextFieldAccessible: Widgets
 

accessible/src/html/HTMLFormControlAccessible.h

@@ -142,8 +142,6 @@ public:
 protected:
   // Accessible
   virtual ENameValueFlag NativeName(nsString& aName) MOZ_OVERRIDE;
-
-  virtual void CacheChildren() MOZ_OVERRIDE;
 };
 
 

accessible/tests/mochitest/editabletext/test_1.html

@@ -70,7 +70,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=452161
       var testRun = new editableTextTestRun();
 
       addTestEditable("input", testRun);
-      addTestEditable("div", testRun, '\n');
+      addTestEditable("div", testRun);
       addTestEditable(getNode("frame").contentDocument, testRun, '\n');
 
       testRun.run(); // Will call SimpleTest.finish();

accessible/tests/mochitest/elm/test_HTMLSpec.html

@@ -1298,7 +1298,8 @@
     <p>This is a quotation taken from the Mozilla Developer Center.</p>
   </blockquote>
 
-  <p id="br_container"><br></p>
+  <!--  two BRs, one will be eaten -->
+  <p id="br_container"><br><br></p>
 
   <button id="button">button</button>
   <form>

accessible/tests/mochitest/text/test_gettext.html

@@ -96,12 +96,12 @@
 
 two words
 </div>
-  <div id="dbr3">oneword<br/><br/>two words<br/></div>
+  <div id="dbr3">oneword<br/><br/>two words<br/><br/></div>
   <div id="e3" contenteditable="true">oneword
 
 two words
 </div>
-  <div id="ebr3" contenteditable="true">oneword<br/><br/>two words<br/></div>
+  <div id="ebr3" contenteditable="true">oneword<br/><br/>two words<br/><br/></div>
   <textarea id="t3" cols="300">oneword
 
 two words

accessible/tests/mochitest/text/test_hypertext.html

@@ -74,7 +74,7 @@
       testCharacterCount(IDs, 5);
       testText(IDs, 0, 5, "1.foo");
 
-      testText(["testbr"], 0, 4, "foo\n");
+      testText(["testbr"], 0, 3, "foo");
 
       testTextAtOffset(2, nsIAccessibleText.BOUNDARY_CHAR, "o", 2, 3, "testbr",
                        kOk, kOk, kOk);

accessible/tests/mochitest/text/test_lineboundary.html

@@ -50,6 +50,7 @@
                              [ 8, 8, "oneword\n", 0, 8 ],
                              [ 9, 18, "\n", 8, 9 ],
                              [ 19, 19, "two words\n", 9, 19 ]]);
+
       testTextBeforeOffset(IDs, BOUNDARY_LINE_END,
                           [ [ 0, 7, "", 0, 0 ],
                             [ 8, 8, "oneword", 0, 7 ],
@@ -96,7 +97,17 @@
 
       testTextAfterOffset([ getAccessible("ht_1").firstChild ], BOUNDARY_LINE_END,
                           [ [ 0, 5, "", 5, 5 ] ]);
-                           
+
+      //////////////////////////////////////////////////////////////////////////
+      // foo<br> and foo<br><br>
+
+      testTextAtOffset([ getAccessible("ht_2").firstChild.firstChild ],
+                       BOUNDARY_LINE_START,
+                       [ [ 0, 3, "foo", 0, 3 ] ]);
+      testTextAtOffset([ getAccessible("ht_3").firstChild.firstChild ],
+                       BOUNDARY_LINE_START,
+                       [ [ 0, 3, "foo\n", 0, 4 ], [ 4, 4, "", 4, 4 ] ]);
+
       SimpleTest.finish();
     }
 
@@ -135,16 +146,16 @@
           src="data:text/html,<html><body><textarea id='ta'>hello my friend</textarea></body></html>"></iframe>
 
   <pre>
-    <div id="ml_div">oneword
+    <div id="ml_div" style="border-style:outset;">oneword
 
 two words
 </div>
-  <div id="ml_divbr">oneword<br/><br/>two words<br/></div>
+  <div id="ml_divbr" style="border-style:outset;">oneword<br/><br/>two words<br/><br/></div>
-  <div id="ml_editable" contenteditable="true">oneword
+  <div id="ml_editable" style="border-style:outset;" contenteditable="true">oneword
 
 two words
 </div>
-  <div id="ml_editablebr" contenteditable="true">oneword<br/><br/>two words<br/></div>
+  <div id="ml_editablebr" contenteditable="true" style="border-style:outset;">oneword<br/><br/>two words<br/><br/></div>
   <textarea id="ml_textarea" cols="300">oneword
 
 two words
@@ -152,5 +163,8 @@ two words
   </pre>
 
   <iframe id="ht_1" src="data:text/html,<html><body>a <a href=''>b</a> c</body></html>"></iframe>
+
+  <iframe id="ht_2" src="data:text/html,<div contentEditable='true'>foo<br/></div>"></iframe>
+  <iframe id="ht_3" src="data:text/html,<div contentEditable='true'>foo<br/><br/></div>"></iframe>
 </body>
 </html>

accessible/tests/mochitest/text/test_wordboundary.html

@@ -268,12 +268,12 @@
 
 two words
 </div>
-<div id="ml_divbr1">oneword<br/><br/>two words<br/></div>
+<div id="ml_divbr1">oneword<br/><br/>two words<br/><br/></div>
 <div id="ml_ediv1" contenteditable="true">oneword
 
 two words
 </div>
-<div id="ml_edivbr1" contenteditable="true">oneword<br/><br/>two words<br/></div>
+<div id="ml_edivbr1" contenteditable="true">oneword<br/><br/>two words<br/><br/></div>
 <textarea id="ml_t1" cols="300">oneword
 
 two words