Bug 1241631. Don't trim whitespace immediately after a soft line break when computing the rendered text for accessibility. r=mats draft
authorRobert O'Callahan <robert@ocallahan.org>
Tue, 01 Mar 2016 12:51:03 +1300
changeset 335612 64140aa868694af281be8ff9ef3986ee7dea418b
parent 335611 db74d44704ea55d419ef257fdaf0f5948dcbd456
child 515178 dfaf9a3999508cadce03f83aefcc2cca9f2f71c1
push id11833
push userrocallahan@mozilla.com
push dateTue, 01 Mar 2016 00:44:12 +0000
reviewersmats
bugs1241631
milestone47.0a1
Bug 1241631. Don't trim whitespace immediately after a soft line break when computing the rendered text for accessibility. r=mats MozReview-Commit-ID: AbADM1UZ7XZ
accessible/base/NotificationController.cpp
accessible/base/nsAccessibilityService.cpp
accessible/base/nsTextEquivUtils.cpp
accessible/generic/Accessible.cpp
accessible/generic/HyperTextAccessible.cpp
accessible/tests/mochitest/text/test_gettext.html
layout/generic/nsIFrame.h
layout/generic/nsTextFrame.cpp
layout/generic/nsTextFrame.h
--- a/accessible/base/NotificationController.cpp
+++ b/accessible/base/NotificationController.cpp
@@ -248,17 +248,17 @@ NotificationController::WillRefresh(mozi
       continue;
     }
 
     nsIContent* containerElm = containerNode->IsElement() ?
       containerNode->AsElement() : nullptr;
 
     nsIFrame::RenderedText text = textFrame->GetRenderedText(0,
         UINT32_MAX, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT,
-        nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE);
+        nsIFrame::RenderWhitespace::TRIM_WHITESPACE_AT_HARD_LINE_BOUNDARIES);
 
     // Remove text accessible if rendered text is empty.
     if (textAcc) {
       if (text.mString.IsEmpty()) {
   #ifdef A11Y_LOG
         if (logging::IsEnabled(logging::eTree | logging::eText)) {
           logging::MsgBegin("TREE", "text node lost its content");
           logging::Node("container", containerElm);
--- a/accessible/base/nsAccessibilityService.cpp
+++ b/accessible/base/nsAccessibilityService.cpp
@@ -1088,17 +1088,17 @@ nsAccessibilityService::GetOrCreateAcces
 
   // Attempt to create an accessible based on what we know.
   RefPtr<Accessible> newAcc;
 
   // Create accessible for visible text frames.
   if (content->IsNodeOfType(nsINode::eTEXT)) {
     nsIFrame::RenderedText text = frame->GetRenderedText(0,
         UINT32_MAX, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT,
-        nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE);
+        nsIFrame::RenderWhitespace::TRIM_WHITESPACE_AT_HARD_LINE_BOUNDARIES);
     // Ignore not rendered text nodes and whitespace text nodes between table
     // cells.
     if (text.mString.IsEmpty() ||
         (aContext->IsTableRow() && nsCoreUtils::IsWhitespaceString(text.mString))) {
       if (aIsSubtreeHidden)
         *aIsSubtreeHidden = true;
 
       return nullptr;
--- a/accessible/base/nsTextEquivUtils.cpp
+++ b/accessible/base/nsTextEquivUtils.cpp
@@ -136,17 +136,17 @@ nsTextEquivUtils::AppendTextEquivFromTex
       }
     }
     
     if (aContent->TextLength() > 0) {
       nsIFrame *frame = aContent->GetPrimaryFrame();
       if (frame) {
         nsIFrame::RenderedText text = frame->GetRenderedText(0,
             UINT32_MAX, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT,
-            nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE);
+            nsIFrame::RenderWhitespace::TRIM_WHITESPACE_AT_HARD_LINE_BOUNDARIES);
         aString->Append(text.mString);
       } else {
         // If aContent is an object that is display: none, we have no a frame.
         aContent->AppendTextTo(*aString);
       }
       if (isHTMLBlock && !aString->IsEmpty()) {
         aString->Append(char16_t(' '));
       }
--- a/accessible/generic/Accessible.cpp
+++ b/accessible/generic/Accessible.cpp
@@ -385,17 +385,17 @@ Accessible::VisibilityState()
   // marked invisible.
   // XXX Can we just remove this check? Why do we need to mark empty
   // text invisible?
   if (frame->GetType() == nsGkAtoms::textFrame &&
       !(frame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) &&
       frame->GetRect().IsEmpty()) {
     nsIFrame::RenderedText text = frame->GetRenderedText(0,
         UINT32_MAX, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT,
-        nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE);
+        nsIFrame::RenderWhitespace::TRIM_WHITESPACE_AT_HARD_LINE_BOUNDARIES);
     if (text.mString.IsEmpty()) {
       return states::INVISIBLE;
     }
   }
 
   return 0;
 }
 
--- a/accessible/generic/HyperTextAccessible.cpp
+++ b/accessible/generic/HyperTextAccessible.cpp
@@ -1956,17 +1956,17 @@ HyperTextAccessible::ContentToRenderedOf
 
   NS_ASSERTION(aFrame->GetType() == nsGkAtoms::textFrame,
                "Need text frame for offset conversion");
   NS_ASSERTION(aFrame->GetPrevContinuation() == nullptr,
                "Call on primary frame only");
 
   nsIFrame::RenderedText text = aFrame->GetRenderedText(aContentOffset,
       aContentOffset + 1, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT,
-      nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE);
+      nsIFrame::RenderWhitespace::TRIM_WHITESPACE_AT_HARD_LINE_BOUNDARIES);
   *aRenderedOffset = text.mOffsetWithinNodeRenderedText;
 
   return NS_OK;
 }
 
 nsresult
 HyperTextAccessible::RenderedToContentOffset(nsIFrame* aFrame, uint32_t aRenderedOffset,
                                              int32_t* aContentOffset) const
@@ -1981,17 +1981,17 @@ HyperTextAccessible::RenderedToContentOf
 
   NS_ASSERTION(aFrame->GetType() == nsGkAtoms::textFrame,
                "Need text frame for offset conversion");
   NS_ASSERTION(aFrame->GetPrevContinuation() == nullptr,
                "Call on primary frame only");
 
   nsIFrame::RenderedText text = aFrame->GetRenderedText(aRenderedOffset,
       aRenderedOffset + 1, nsIFrame::TextOffsetType::OFFSETS_IN_RENDERED_TEXT,
-      nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE);
+      nsIFrame::RenderWhitespace::TRIM_WHITESPACE_AT_HARD_LINE_BOUNDARIES);
   *aContentOffset = text.mOffsetWithinNodeText;
 
   return NS_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // HyperTextAccessible public
 
--- a/accessible/tests/mochitest/text/test_gettext.html
+++ b/accessible/tests/mochitest/text/test_gettext.html
@@ -15,17 +15,17 @@
   <script type="application/javascript">
     function doTest()
     {
       //////////////////////////////////////////////////////////////////////////
       //
       // __h__e__l__l__o__ __m__y__ __f__r__i__e__n__d__
       //  0  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15
 
-      var IDs = [ "i1", "d1", "e1", "t1" ];
+      var IDs = [ "i1", "d1", "e1", "t1", "d4" ];
 
       testCharacterCount(IDs, 15);
 
       testText(IDs, 0, 1, "h");
       testText(IDs, 1, 3, "el");
       testText(IDs, 14, 15, "d");
       testText(IDs, 0, 15, "hello my friend");
       testText(IDs, 0, -1, "hello my friend");
@@ -78,16 +78,17 @@
   <div id="content" style="display: none"></div>
   <pre id="test">
   </pre>
 
   <input id="i1" value="hello my friend"/>
   <div id="d1">hello my friend</div>
   <div id="e1" contenteditable="true">hello my friend</div>
   <textarea id="t1">hello my friend</textarea>
+  <div id="d4" style="word-wrap:break-word; width:1px">hello my friend</div>
 
   <input id="i2" value="Brave Sir  Robin   ran"/>
   <pre><div id="dpre2">Brave Sir  Robin   ran</div></pre>
   <pre><div id="epre2" contenteditable="true">Brave Sir  Robin   ran</div></pre>
   <textarea id="t2" cols="300">Brave Sir  Robin   ran</textarea>
   <div id="d2">Brave Sir  Robin   ran</div>
   <div id="e2" contenteditable="true">Brave Sir  Robin   ran</div>
 
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -1957,27 +1957,28 @@ public:
         mOffsetWithinNodeText(0) {}
   };
   enum class TextOffsetType {
     // Passed-in start and end offsets are within the content text.
     OFFSETS_IN_CONTENT_TEXT,
     // Passed-in start and end offsets are within the rendered text.
     OFFSETS_IN_RENDERED_TEXT
   };
-  enum class TrailingWhitespace {
-    TRIM_TRAILING_WHITESPACE,
-    // Spaces preceding a caret at the end of a line should not be trimmed
-    DONT_TRIM_TRAILING_WHITESPACE
+  enum class RenderWhitespace {
+    // Trim whitespace adjacent to hard and soft line breaks.
+    TRIM_WHITESPACE_AT_LINE_BOUNDARIES,
+    // Trim whitespace adjacent to hard line breaks.
+    TRIM_WHITESPACE_AT_HARD_LINE_BOUNDARIES
   };
   virtual RenderedText GetRenderedText(uint32_t aStartOffset = 0,
                                        uint32_t aEndOffset = UINT32_MAX,
                                        TextOffsetType aOffsetType =
                                            TextOffsetType::OFFSETS_IN_CONTENT_TEXT,
-                                       TrailingWhitespace aTrimTrailingWhitespace =
-                                           TrailingWhitespace::TRIM_TRAILING_WHITESPACE)
+                                       RenderWhitespace aTrimTrailingWhitespace =
+                                           RenderWhitespace::TRIM_WHITESPACE_AT_LINE_BOUNDARIES)
   { return RenderedText(); }
 
   /**
    * Returns true if the frame contains any non-collapsed characters.
    * This method is only available for text frames, and it will return false
    * for all other frame types.
    */
   virtual bool HasAnyNoncollapsedCharacters()
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -4058,17 +4058,17 @@ nsTextPaintStyle::GetResolvedForeColor(n
 
 #ifdef ACCESSIBILITY
 a11y::AccType
 nsTextFrame::AccessibleType()
 {
   if (IsEmpty()) {
     RenderedText text = GetRenderedText(0,
         UINT32_MAX, TextOffsetType::OFFSETS_IN_CONTENT_TEXT,
-        TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE);
+        RenderWhitespace::TRIM_WHITESPACE_AT_HARD_LINE_BOUNDARIES);
     if (text.mString.IsEmpty()) {
       return a11y::eNoType;
     }
   }
 
   return a11y::eTextLeafType;
 }
 #endif
@@ -9206,16 +9206,39 @@ static void TransformChars(nsTextFrame* 
         out[i] = ToTitleCase(out[i]);
       }
     }
     break;
   }
 }
 
 static bool
+LineStartsAtHardLineBreak(nsTextFrame* aFrame)
+{
+  nsIFrame* lineContainer = FindLineContainer(aFrame);
+  nsBlockFrame* block = do_QueryFrame(lineContainer);
+  if (!block) {
+    // Weird situation where we have a line layout without a block.
+    // No soft breaks occur in this situation.
+    return true;
+  }
+  bool foundValidLine;
+  nsBlockInFlowLineIterator iter(block, aFrame, &foundValidLine);
+  if (!foundValidLine) {
+    NS_ERROR("Invalid line!");
+    return true;
+  }
+  if (!iter.Prev()) {
+    // Hit block boundary
+    return true;
+  }
+  return !iter.GetLine()->IsLineWrapped();
+}
+
+static bool
 LineEndsInHardLineBreak(nsTextFrame* aFrame)
 {
   nsIFrame* lineContainer = FindLineContainer(aFrame);
   nsBlockFrame* block = do_QueryFrame(lineContainer);
   if (!block) {
     // Weird situation where we have a line layout without a block.
     // No soft breaks occur in this situation.
     return true;
@@ -9228,17 +9251,17 @@ LineEndsInHardLineBreak(nsTextFrame* aFr
   }
   return !iter.GetLine()->IsLineWrapped();
 }
 
 nsIFrame::RenderedText
 nsTextFrame::GetRenderedText(uint32_t aStartOffset,
                              uint32_t aEndOffset,
                              TextOffsetType aOffsetType,
-                             TrailingWhitespace aTrimTrailingWhitespace)
+                             RenderWhitespace aTrimTrailingWhitespace)
 {
   NS_ASSERTION(!GetPrevContinuation(), "Must be called on first-in-flow");
 
   // The handling of offsets could be more efficient...
   RenderedText result;
   nsTextFrame* textFrame;
   const nsTextFragment* textFrag = mContent->GetText();
   uint32_t offsetInRenderedString = 0;
@@ -9255,19 +9278,24 @@ nsTextFrame::GetRenderedText(uint32_t aS
     gfxSkipCharsIterator iter =
       textFrame->EnsureTextRun(nsTextFrame::eInflated);
     if (!textFrame->mTextRun) {
       break;
     }
     gfxSkipCharsIterator tmpIter = iter;
 
     // Skip to the start of the text run, past ignored chars at start of line
-    uint32_t flags = TRIM_BEFORE | TRIM_POST_REFLOW;
+    uint32_t flags = TRIM_POST_REFLOW;
+    if (textFrame->IsAtBeginningOfLine() &&
+        LineStartsAtHardLineBreak(textFrame) &&
+        aTrimTrailingWhitespace == RenderWhitespace::TRIM_WHITESPACE_AT_LINE_BOUNDARIES) {
+      flags |= TRIM_BEFORE;
+    }
     if (textFrame->IsAtEndOfLine() && LineEndsInHardLineBreak(textFrame) &&
-        aTrimTrailingWhitespace == TrailingWhitespace::TRIM_TRAILING_WHITESPACE) {
+        aTrimTrailingWhitespace == RenderWhitespace::TRIM_WHITESPACE_AT_LINE_BOUNDARIES) {
       flags |= TRIM_AFTER;
     }
     TrimmedOffsets trimmedOffsets =
         textFrame->GetTrimmedOffsets(textFrag, flags);
     bool trimmedSignificantNewline =
         trimmedOffsets.GetEnd() < GetContentEnd() &&
         HasSignificantTerminalNewline();
     uint32_t skippedToRenderedStringOffset = offsetInRenderedString -
@@ -9521,16 +9549,22 @@ nsTextFrame::HasSignificantTerminalNewli
 }
 
 bool
 nsTextFrame::IsAtEndOfLine() const
 {
   return (GetStateBits() & TEXT_END_OF_LINE) != 0;
 }
 
+bool
+nsTextFrame::IsAtBeginningOfLine() const
+{
+  return (GetStateBits() & TEXT_START_OF_LINE) != 0;
+}
+
 nscoord
 nsTextFrame::GetLogicalBaseline(WritingMode aWritingMode ) const
 {
   return mAscent;
 }
 
 bool
 nsTextFrame::HasAnyNoncollapsedCharacters()
--- a/layout/generic/nsTextFrame.h
+++ b/layout/generic/nsTextFrame.h
@@ -202,16 +202,21 @@ public:
   
   virtual bool HasSignificantTerminalNewline() const override;
 
   /**
    * Returns true if this text frame is logically adjacent to the end of the
    * line.
    */
   bool IsAtEndOfLine() const;
+  /**
+   * Returns true if this text frame is logically adjacent to the
+   * beginning of the line.
+   */
+  bool IsAtBeginningOfLine() const;
   
   /**
    * Call this only after reflow the frame. Returns true if non-collapsed
    * characters are present.
    */
   bool HasNoncollapsedCharacters() const {
     return (GetStateBits() & TEXT_HAS_NONCOLLAPSED_CHARACTERS) != 0;
   }
@@ -262,18 +267,18 @@ public:
     // an amount to *subtract* from the frame's width (zero if !mChanged)
     nscoord      mDeltaWidth;
   };
   TrimOutput TrimTrailingWhiteSpace(DrawTarget* aDrawTarget);
   virtual RenderedText GetRenderedText(uint32_t aStartOffset = 0,
                                        uint32_t aEndOffset = UINT32_MAX,
                                        TextOffsetType aOffsetType =
                                            TextOffsetType::OFFSETS_IN_CONTENT_TEXT,
-                                       TrailingWhitespace aTrimTrailingWhitespace =
-                                           TrailingWhitespace::TRIM_TRAILING_WHITESPACE) override;
+                                       RenderWhitespace aTrimTrailingWhitespace =
+                                           RenderWhitespace::TRIM_WHITESPACE_AT_LINE_BOUNDARIES) override;
 
   nsOverflowAreas RecomputeOverflow(nsIFrame* aBlockFrame);
 
   enum TextRunType {
     // Anything in reflow (but not intrinsic width calculation) or
     // painting should use the inflated text run (i.e., with font size
     // inflation applied).
     eInflated,