From 884728e2c128c4b1ae03054b7d2766f4e58be1be Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Tue, 1 Dec 2015 02:21:25 +1300 Subject: [PATCH] Bug 1221043. Revert to including trailing whitespace for accessibility APIs. r=marcoz,mats --- accessible/base/NotificationController.cpp | 4 +++- accessible/base/nsAccessibilityService.cpp | 4 +++- accessible/base/nsTextEquivUtils.cpp | 4 +++- accessible/generic/Accessible.cpp | 4 +++- accessible/generic/HyperTextAccessible.cpp | 6 ++++-- .../tests/mochitest/jsat/test_traversal.html | 4 ++-- .../mochitest/pivot/test_virtualcursor.html | 4 ++-- accessible/tests/mochitest/text/test_doc.html | 4 ++-- .../tests/mochitest/text/test_hypertext.html | 2 +- .../mochitest/text/test_lineboundary.html | 10 +++++----- .../mochitest/textattrs/test_general.html | 18 +++++++++--------- .../mochitest/textcaret/test_general.html | 2 +- .../tests/mochitest/tree/test_txtcntr.html | 8 ++++---- layout/generic/nsIFrame.h | 9 ++++++++- layout/generic/nsTextFrame.cpp | 10 +++++++--- layout/generic/nsTextFrame.h | 4 +++- 16 files changed, 60 insertions(+), 37 deletions(-) diff --git a/accessible/base/NotificationController.cpp b/accessible/base/NotificationController.cpp index 33e2d435527..ec7d7cd46ce 100644 --- a/accessible/base/NotificationController.cpp +++ b/accessible/base/NotificationController.cpp @@ -230,7 +230,9 @@ NotificationController::WillRefresh(mozilla::TimeStamp aTime) nsIContent* containerElm = containerNode->IsElement() ? containerNode->AsElement() : nullptr; - nsIFrame::RenderedText text = textFrame->GetRenderedText(); + nsIFrame::RenderedText text = textFrame->GetRenderedText(0, + UINT32_MAX, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT, + nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE); // Remove text accessible if rendered text is empty. if (textAcc) { diff --git a/accessible/base/nsAccessibilityService.cpp b/accessible/base/nsAccessibilityService.cpp index 95cf3f6fb72..10b316c3957 100644 --- a/accessible/base/nsAccessibilityService.cpp +++ b/accessible/base/nsAccessibilityService.cpp @@ -1091,7 +1091,9 @@ nsAccessibilityService::GetOrCreateAccessible(nsINode* aNode, // Create accessible for visible text frames. if (content->IsNodeOfType(nsINode::eTEXT)) { - nsIFrame::RenderedText text = frame->GetRenderedText(); + nsIFrame::RenderedText text = frame->GetRenderedText(0, + UINT32_MAX, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT, + nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE); // Ignore not rendered text nodes and whitespace text nodes between table // cells. if (text.mString.IsEmpty() || diff --git a/accessible/base/nsTextEquivUtils.cpp b/accessible/base/nsTextEquivUtils.cpp index bb7b3d918de..b7e703c2f08 100644 --- a/accessible/base/nsTextEquivUtils.cpp +++ b/accessible/base/nsTextEquivUtils.cpp @@ -139,7 +139,9 @@ nsTextEquivUtils::AppendTextEquivFromTextContent(nsIContent *aContent, if (aContent->TextLength() > 0) { nsIFrame *frame = aContent->GetPrimaryFrame(); if (frame) { - nsIFrame::RenderedText text = frame->GetRenderedText(); + nsIFrame::RenderedText text = frame->GetRenderedText(0, + UINT32_MAX, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT, + nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE); aString->Append(text.mString); } else { // If aContent is an object that is display: none, we have no a frame. diff --git a/accessible/generic/Accessible.cpp b/accessible/generic/Accessible.cpp index 0b8f5368999..05ee9a9db21 100644 --- a/accessible/generic/Accessible.cpp +++ b/accessible/generic/Accessible.cpp @@ -394,7 +394,9 @@ Accessible::VisibilityState() if (frame->GetType() == nsGkAtoms::textFrame && !(frame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) && frame->GetRect().IsEmpty()) { - nsIFrame::RenderedText text = frame->GetRenderedText(); + nsIFrame::RenderedText text = frame->GetRenderedText(0, + UINT32_MAX, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT, + nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE); if (text.mString.IsEmpty()) { return states::INVISIBLE; } diff --git a/accessible/generic/HyperTextAccessible.cpp b/accessible/generic/HyperTextAccessible.cpp index 76b8fe49caa..b6f47dca8a1 100644 --- a/accessible/generic/HyperTextAccessible.cpp +++ b/accessible/generic/HyperTextAccessible.cpp @@ -1985,7 +1985,8 @@ HyperTextAccessible::ContentToRenderedOffset(nsIFrame* aFrame, int32_t aContentO "Call on primary frame only"); nsIFrame::RenderedText text = aFrame->GetRenderedText(aContentOffset, - aContentOffset + 1); + aContentOffset + 1, nsIFrame::TextOffsetType::OFFSETS_IN_CONTENT_TEXT, + nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE); *aRenderedOffset = text.mOffsetWithinNodeRenderedText; return NS_OK; @@ -2009,7 +2010,8 @@ HyperTextAccessible::RenderedToContentOffset(nsIFrame* aFrame, uint32_t aRendere "Call on primary frame only"); nsIFrame::RenderedText text = aFrame->GetRenderedText(aRenderedOffset, - aRenderedOffset + 1, nsIFrame::TextOffsetType::OFFSETS_IN_RENDERED_TEXT); + aRenderedOffset + 1, nsIFrame::TextOffsetType::OFFSETS_IN_RENDERED_TEXT, + nsIFrame::TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE); *aContentOffset = text.mOffsetWithinNodeText; return NS_OK; diff --git a/accessible/tests/mochitest/jsat/test_traversal.html b/accessible/tests/mochitest/jsat/test_traversal.html index 12316034aea..533e9d89ff9 100644 --- a/accessible/tests/mochitest/jsat/test_traversal.html +++ b/accessible/tests/mochitest/jsat/test_traversal.html @@ -113,7 +113,7 @@ 'A esoteric weapon wielded by only the most ' + 'formidable warriors, for its unrelenting strict' + ' power is unfathomable.', - '• Lists of Programming Languages', 'Lisp', + '• Lists of Programming Languages', 'Lisp ', '1. Scheme', '2. Racket', '3. Clojure', '4. Standard Lisp', 'link-0', ' Lisp', 'checkbox-1-5', ' LeLisp', '• JavaScript', @@ -124,7 +124,7 @@ '5 8', 'gridcell4', 'Just an innocuous separator', 'Dirty Words', 'Meaning', 'Mud', 'Wet Dirt', 'Dirt', 'Messy Stuff', 'statusbar-1', 'statusbar-2', - 'switch-1', 'This is a MathML formula', 'math-1', + 'switch-1', 'This is a MathML formula ', 'math-1', 'with some text after.']); queueTraversalSequence(gQueue, docAcc, TraversalRules.Landmark, null, diff --git a/accessible/tests/mochitest/pivot/test_virtualcursor.html b/accessible/tests/mochitest/pivot/test_virtualcursor.html index 9bb17348434..2fb339964af 100644 --- a/accessible/tests/mochitest/pivot/test_virtualcursor.html +++ b/accessible/tests/mochitest/pivot/test_virtualcursor.html @@ -52,7 +52,7 @@ gQueue, docAcc, ObjectTraversalRule, null, ['Main Title', 'Lorem ipsum ', 'dolor', ' sit amet. Integer vitae urna leo, id ', - 'semper', ' nulla.', 'Second Section Title', + 'semper', ' nulla. ', 'Second Section Title', 'Sed accumsan luctus lacus, vitae mollis arcu tristique vulputate.', 'An ', 'embedded', ' document.', 'Hide me', 'Link 1', 'Link 2', 'Link 3', 'Hello', 'World']); @@ -90,7 +90,7 @@ gQueue, docAcc, ObjectTraversalRule, getAccessible(doc.getElementById('paragraph-1')), ['Lorem ipsum ', 'dolor', ' sit amet. Integer vitae urna leo, id ', - 'semper', ' nulla.']); + 'semper', ' nulla. ']); gQueue.push(new setModalRootInvoker(docAcc, docAcc.parent, NS_ERROR_INVALID_ARG)); diff --git a/accessible/tests/mochitest/text/test_doc.html b/accessible/tests/mochitest/text/test_doc.html index 76b801c277f..9c6788275a0 100644 --- a/accessible/tests/mochitest/text/test_doc.html +++ b/accessible/tests/mochitest/text/test_doc.html @@ -16,8 +16,8 @@ function doTest() { var iframeDoc = [ getNode("iframe").contentDocument ]; - testCharacterCount(iframeDoc, 13); - testText(iframeDoc, 0, 13, "outbodyinbody"); + testCharacterCount(iframeDoc, 15); + testText(iframeDoc, 0, 15, "outbody inbody "); SimpleTest.finish(); } diff --git a/accessible/tests/mochitest/text/test_hypertext.html b/accessible/tests/mochitest/text/test_hypertext.html index 6df2419aa6f..2d71e11a9d8 100644 --- a/accessible/tests/mochitest/text/test_hypertext.html +++ b/accessible/tests/mochitest/text/test_hypertext.html @@ -58,7 +58,7 @@ //////////////////////////////////////////////////////////////////////// // getTextAtOffset line boundary - testTextAtOffset(0, BOUNDARY_LINE_START, "line", 0, 4, + testTextAtOffset(0, BOUNDARY_LINE_START, "line ", 0, 5, "hypertext3", kOk, kOk, kOk); // XXX: see bug 634202. diff --git a/accessible/tests/mochitest/text/test_lineboundary.html b/accessible/tests/mochitest/text/test_lineboundary.html index ff1364b9ba6..e2e170e2f44 100644 --- a/accessible/tests/mochitest/text/test_lineboundary.html +++ b/accessible/tests/mochitest/text/test_lineboundary.html @@ -14,9 +14,9 @@ function doTest() { testTextAtOffset("line_test_1", BOUNDARY_LINE_START, - [[0, 5, "Line 1", 0, 6], - [6, 6, "", 6, 6], - [7, 13, "Line 3", 7, 13]]); + [[0, 6, "Line 1 ", 0, 7], + [7, 7, "", 7, 7], + [8, 15, "Line 3 ", 8, 15]]); ////////////////////////////////////////////////////////////////////////// // __h__e__l__l__o__ __m__y__ __f__r__i__e__n__d__ @@ -114,10 +114,10 @@ [ [ 0, 3, "foo\n", 0, 4 ], [ 4, 4, "", 4, 4 ] ]); ////////////////////////////////////////////////////////////////////////// - // 'Hello world' + // 'Hello world ' (\n is rendered as space) testTextAtOffset([ "ht_4" ], BOUNDARY_LINE_START, - [ [ 0, 11, "Hello world", 0, 11 ] ]); + [ [ 0, 12, "Hello world ", 0, 12 ] ]); ////////////////////////////////////////////////////////////////////////// // list items diff --git a/accessible/tests/mochitest/textattrs/test_general.html b/accessible/tests/mochitest/textattrs/test_general.html index 4eebe604a0b..142701b175c 100644 --- a/accessible/tests/mochitest/textattrs/test_general.html +++ b/accessible/tests/mochitest/textattrs/test_general.html @@ -89,7 +89,7 @@ gComputedStyle = document.defaultView.getComputedStyle(tempElem, ""); attrs = {"color": gComputedStyle.color, "background-color": gComputedStyle.backgroundColor}; - testTextAttrs(ID, 27, attrs, defAttrs, 27, 49); + testTextAttrs(ID, 27, attrs, defAttrs, 27, 50); ////////////////////////////////////////////////////////////////////////// // area4 @@ -110,7 +110,7 @@ tempElem = tempElem.parentNode; gComputedStyle = document.defaultView.getComputedStyle(tempElem, ""); attrs = {"color": gComputedStyle.color}; - testTextAttrs(ID, 34, attrs, defAttrs, 33, 45); + testTextAttrs(ID, 34, attrs, defAttrs, 33, 46); ////////////////////////////////////////////////////////////////////////// // area5: "Green!*!RedNormal" @@ -144,7 +144,7 @@ // Normal attrs = {}; - testTextAttrs(ID, 11, attrs, defAttrs, 11, 17); + testTextAttrs(ID, 11, attrs, defAttrs, 11, 18); ////////////////////////////////////////////////////////////////////////// // area6 (CSS vertical-align property, refer to bug 445938 for details @@ -323,7 +323,7 @@ testTextAttrs(ID, 152, attrs, defAttrs, 151, 164); attrs = {}; - testTextAttrs(ID, 165, attrs, defAttrs, 164, 171); + testTextAttrs(ID, 165, attrs, defAttrs, 164, 172); ////////////////////////////////////////////////////////////////////////// // area10, different single style spans in non-styled paragraph @@ -383,7 +383,7 @@ testTextAttrs(ID, 111, attrs, defAttrs, 110, 123); attrs = {}; - testTextAttrs(ID, 124, attrs, defAttrs, 123, 130); + testTextAttrs(ID, 124, attrs, defAttrs, 123, 131); ////////////////////////////////////////////////////////////////////////// // area11, "font-weight" tests @@ -410,7 +410,7 @@ testTextAttrs(ID, 51, attrs, defAttrs, 51, 57); attrs = { }; - testTextAttrs(ID, 57, attrs, defAttrs, 57, 96); + testTextAttrs(ID, 57, attrs, defAttrs, 57, 97); ////////////////////////////////////////////////////////////////////////// // test out of range offset @@ -487,7 +487,7 @@ testTextAttrs(ID, 27, attrs, defAttrs, 27, 31); attrs = { }; - testTextAttrs(ID, 31, attrs, defAttrs, 31, 44); + testTextAttrs(ID, 31, attrs, defAttrs, 31, 45); } ////////////////////////////////////////////////////////////////////////// @@ -530,7 +530,7 @@ "text-line-through-style": "wavy", "text-line-through-color": "rgb(0, 0, 0)", }; - testTextAttrs(ID, 39, attrs, defAttrs, 39, 43); + testTextAttrs(ID, 39, attrs, defAttrs, 39, 44); ////////////////////////////////////////////////////////////////////////// // area18, "auto-generation text" tests @@ -560,7 +560,7 @@ testTextAttrs(ID, 11, attrs, defAttrs, 10, 17); attrs = {}; - testTextAttrs(ID, 18, attrs, defAttrs, 17, 27); + testTextAttrs(ID, 18, attrs, defAttrs, 17, 28); ////////////////////////////////////////////////////////////////////////// // area20, "aOffset as -1 (Mozilla Bug 789621)" test diff --git a/accessible/tests/mochitest/textcaret/test_general.html b/accessible/tests/mochitest/textcaret/test_general.html index 903bb162657..69f83959f18 100644 --- a/accessible/tests/mochitest/textcaret/test_general.html +++ b/accessible/tests/mochitest/textcaret/test_general.html @@ -71,7 +71,7 @@ turnCaretBrowsing(true); // test caret offsets - testCaretOffset(document, 15); + testCaretOffset(document, 16); testCaretOffset("textbox", -1); testCaretOffset("textarea", -1); testCaretOffset("p", -1); diff --git a/accessible/tests/mochitest/tree/test_txtcntr.html b/accessible/tests/mochitest/tree/test_txtcntr.html index 54e42b7f375..80a225ce785 100644 --- a/accessible/tests/mochitest/tree/test_txtcntr.html +++ b/accessible/tests/mochitest/tree/test_txtcntr.html @@ -49,14 +49,14 @@ }, { role: ROLE_TEXT_LEAF, - name: "Hello3" + name: "Hello3 " }, { role: ROLE_PARAGRAPH, children: [ { role: ROLE_TEXT_LEAF, - name: "Hello4" + name: "Hello4 " } ] } @@ -71,7 +71,7 @@ children: [ { role: ROLE_TEXT_LEAF, - name: "helllo" + name: "helllo " }, { role: ROLE_PARAGRAPH, @@ -84,7 +84,7 @@ }, { role: ROLE_TEXT_LEAF, - name: "hello" + name: "hello " } ] }; diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index d7ac65d1439..ab6115dd546 100644 --- a/layout/generic/nsIFrame.h +++ b/layout/generic/nsIFrame.h @@ -1938,10 +1938,17 @@ public: // 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 + }; virtual RenderedText GetRenderedText(uint32_t aStartOffset = 0, uint32_t aEndOffset = UINT32_MAX, TextOffsetType aOffsetType = - TextOffsetType::OFFSETS_IN_CONTENT_TEXT) + TextOffsetType::OFFSETS_IN_CONTENT_TEXT, + TrailingWhitespace aTrimTrailingWhitespace = + TrailingWhitespace::TRIM_TRAILING_WHITESPACE) { return RenderedText(); } /** diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index ee51495901c..b0bb986f7b5 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -4044,7 +4044,9 @@ a11y::AccType nsTextFrame::AccessibleType() { if (IsEmpty()) { - RenderedText text = GetRenderedText(); + RenderedText text = GetRenderedText(0, + UINT32_MAX, TextOffsetType::OFFSETS_IN_CONTENT_TEXT, + TrailingWhitespace::DONT_TRIM_TRAILING_WHITESPACE); if (text.mString.IsEmpty()) { return a11y::eNoType; } @@ -9197,7 +9199,8 @@ LineEndsInHardLineBreak(nsTextFrame* aFrame) nsIFrame::RenderedText nsTextFrame::GetRenderedText(uint32_t aStartOffset, uint32_t aEndOffset, - TextOffsetType aOffsetType) + TextOffsetType aOffsetType, + TrailingWhitespace aTrimTrailingWhitespace) { NS_ASSERTION(!GetPrevContinuation(), "Must be called on first-in-flow"); @@ -9225,7 +9228,8 @@ nsTextFrame::GetRenderedText(uint32_t aStartOffset, // Skip to the start of the text run, past ignored chars at start of line TrimmedOffsets trimmedOffsets = textFrame->GetTrimmedOffsets(textFrag, - textFrame->IsAtEndOfLine() && LineEndsInHardLineBreak(textFrame)); + textFrame->IsAtEndOfLine() && LineEndsInHardLineBreak(textFrame) && + aTrimTrailingWhitespace == TrailingWhitespace::TRIM_TRAILING_WHITESPACE); bool trimmedSignificantNewline = trimmedOffsets.GetEnd() < GetContentEnd() && HasSignificantTerminalNewline(); diff --git a/layout/generic/nsTextFrame.h b/layout/generic/nsTextFrame.h index caef123e66c..3cb5a0404b0 100644 --- a/layout/generic/nsTextFrame.h +++ b/layout/generic/nsTextFrame.h @@ -266,7 +266,9 @@ public: virtual RenderedText GetRenderedText(uint32_t aStartOffset = 0, uint32_t aEndOffset = UINT32_MAX, TextOffsetType aOffsetType = - TextOffsetType::OFFSETS_IN_CONTENT_TEXT) override; + TextOffsetType::OFFSETS_IN_CONTENT_TEXT, + TrailingWhitespace aTrimTrailingWhitespace = + TrailingWhitespace::TRIM_TRAILING_WHITESPACE) override; nsOverflowAreas RecomputeOverflow(nsIFrame* aBlockFrame);