Bug 264412. Refactor nsIFrame::GetRenderedText API to be more sane. r=mats,marcoz
authorRobert O'Callahan <robert@ocallahan.org>
Sat, 24 Oct 2015 22:27:29 +1300
changeset 304551 ab657569f554a1b1dae9575fb95ca5a94949442a
parent 304550 2c2f35c908d08dffa9cd95dbc47de54b3fd76ebf
child 304552 a396f4262479a5695c918f59bacd542ba21448b0
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats, marcoz
bugs264412
milestone44.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 264412. Refactor nsIFrame::GetRenderedText API to be more sane. r=mats,marcoz The test changes here are to adjust for the fact that nsTextFrame::GetRenderedText can now trim whitespace from the end of lines that end in a hard line break.
accessible/base/NotificationController.cpp
accessible/base/nsAccessibilityService.cpp
accessible/base/nsTextEquivUtils.cpp
accessible/generic/Accessible.cpp
accessible/generic/HyperTextAccessible.cpp
accessible/tests/mochitest/jsat/test_traversal.html
accessible/tests/mochitest/pivot/test_virtualcursor.html
accessible/tests/mochitest/text/test_doc.html
accessible/tests/mochitest/text/test_hypertext.html
accessible/tests/mochitest/text/test_lineboundary.html
accessible/tests/mochitest/text/test_wordboundary.html
accessible/tests/mochitest/tree/test_txtcntr.html
gfx/tests/gtest/TestSkipChars.cpp
gfx/thebes/gfxSkipChars.h
layout/generic/nsIFrame.h
layout/generic/nsTextFrame.cpp
layout/generic/nsTextFrame.h
--- a/accessible/base/NotificationController.cpp
+++ b/accessible/base/NotificationController.cpp
@@ -223,22 +223,21 @@ NotificationController::WillRefresh(mozi
       NS_ASSERTION(!textAcc,
                    "Text node isn't rendered but accessible is kept alive!");
       continue;
     }
 
     nsIContent* containerElm = containerNode->IsElement() ?
       containerNode->AsElement() : nullptr;
 
-    nsAutoString text;
-    textFrame->GetRenderedText(&text);
+    nsIFrame::RenderedText text = textFrame->GetRenderedText();
 
     // Remove text accessible if rendered text is empty.
     if (textAcc) {
-      if (text.IsEmpty()) {
+      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);
           logging::Node("content", textNode);
           logging::MsgEnd();
         }
   #endif
@@ -251,27 +250,27 @@ NotificationController::WillRefresh(mozi
   #ifdef A11Y_LOG
       if (logging::IsEnabled(logging::eText)) {
         logging::MsgBegin("TEXT", "text may be changed");
         logging::Node("container", containerElm);
         logging::Node("content", textNode);
         logging::MsgEntry("old text '%s'",
                           NS_ConvertUTF16toUTF8(textAcc->AsTextLeaf()->Text()).get());
         logging::MsgEntry("new text: '%s'",
-                          NS_ConvertUTF16toUTF8(text).get());
+                          NS_ConvertUTF16toUTF8(text.mString).get());
         logging::MsgEnd();
       }
   #endif
 
-      TextUpdater::Run(mDocument, textAcc->AsTextLeaf(), text);
+      TextUpdater::Run(mDocument, textAcc->AsTextLeaf(), text.mString);
       continue;
     }
 
     // Append an accessible if rendered text is not empty.
-    if (!text.IsEmpty()) {
+    if (!text.mString.IsEmpty()) {
   #ifdef A11Y_LOG
       if (logging::IsEnabled(logging::eTree | logging::eText)) {
         logging::MsgBegin("TREE", "text node gains new content");
         logging::Node("container", containerElm);
         logging::Node("content", textNode);
         logging::MsgEnd();
       }
   #endif
--- a/accessible/base/nsAccessibilityService.cpp
+++ b/accessible/base/nsAccessibilityService.cpp
@@ -1086,34 +1086,33 @@ nsAccessibilityService::GetOrCreateAcces
                "Image map manages the area accessible creation!");
 #endif
 
   // Attempt to create an accessible based on what we know.
   RefPtr<Accessible> newAcc;
 
   // Create accessible for visible text frames.
   if (content->IsNodeOfType(nsINode::eTEXT)) {
-    nsAutoString text;
-    frame->GetRenderedText(&text, nullptr, nullptr, 0, UINT32_MAX);
+    nsIFrame::RenderedText text = frame->GetRenderedText();
     // Ignore not rendered text nodes and whitespace text nodes between table
     // cells.
-    if (text.IsEmpty() ||
-        (aContext->IsTableRow() && nsCoreUtils::IsWhitespaceString(text))) {
+    if (text.mString.IsEmpty() ||
+        (aContext->IsTableRow() && nsCoreUtils::IsWhitespaceString(text.mString))) {
       if (aIsSubtreeHidden)
         *aIsSubtreeHidden = true;
 
       return nullptr;
     }
 
     newAcc = CreateAccessibleByFrameType(frame, content, aContext);
     if (!aContext->IsAcceptableChild(newAcc))
       return nullptr;
 
     document->BindToDocument(newAcc, nullptr);
-    newAcc->AsTextLeaf()->SetText(text);
+    newAcc->AsTextLeaf()->SetText(text.mString);
     return newAcc;
   }
 
   if (content->IsHTMLElement(nsGkAtoms::map)) {
     // Create hyper text accessible for HTML map if it is used to group links
     // (see http://www.w3.org/TR/WCAG10-HTML-TECHS/#group-bypass). If the HTML
     // map rect is empty then it is used for links grouping. Otherwise it should
     // be used in conjunction with HTML image element and in this case we don't
--- a/accessible/base/nsTextEquivUtils.cpp
+++ b/accessible/base/nsTextEquivUtils.cpp
@@ -134,18 +134,18 @@ nsTextEquivUtils::AppendTextEquivFromTex
           }
         }
       }
     }
     
     if (aContent->TextLength() > 0) {
       nsIFrame *frame = aContent->GetPrimaryFrame();
       if (frame) {
-        nsresult rv = frame->GetRenderedText(aString);
-        NS_ENSURE_SUCCESS(rv, rv);
+        nsIFrame::RenderedText text = frame->GetRenderedText();
+        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
@@ -389,20 +389,20 @@ Accessible::VisibilityState()
   // Zero area rects can occur in the first frame of a multi-frame text flow,
   // in which case the rendered text is not empty and the frame should not be
   // 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()) {
-    nsAutoString renderedText;
-    frame->GetRenderedText(&renderedText, nullptr, nullptr, 0, 1);
-    if (renderedText.IsEmpty())
+    nsIFrame::RenderedText text = frame->GetRenderedText();
+    if (text.mString.IsEmpty()) {
       return states::INVISIBLE;
+    }
   }
 
   return 0;
 }
 
 uint64_t
 Accessible::NativeState()
 {
--- a/accessible/generic/HyperTextAccessible.cpp
+++ b/accessible/generic/HyperTextAccessible.cpp
@@ -1979,27 +1979,19 @@ HyperTextAccessible::ContentToRenderedOf
     return NS_OK;
   }
 
   NS_ASSERTION(aFrame->GetType() == nsGkAtoms::textFrame,
                "Need text frame for offset conversion");
   NS_ASSERTION(aFrame->GetPrevContinuation() == nullptr,
                "Call on primary frame only");
 
-  gfxSkipChars skipChars;
-  gfxSkipCharsIterator iter;
-  // Only get info up to original offset, we know that will be larger than skipped offset
-  nsresult rv = aFrame->GetRenderedText(nullptr, &skipChars, &iter, 0, aContentOffset);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  uint32_t ourRenderedStart = iter.GetSkippedOffset();
-  int32_t ourContentStart = iter.GetOriginalOffset();
-
-  *aRenderedOffset = iter.ConvertOriginalToSkipped(aContentOffset + ourContentStart) -
-                    ourRenderedStart;
+  nsIFrame::RenderedText text = aFrame->GetRenderedText(aContentOffset,
+      aContentOffset + 1);
+  *aRenderedOffset = text.mOffsetWithinNodeRenderedText;
 
   return NS_OK;
 }
 
 nsresult
 HyperTextAccessible::RenderedToContentOffset(nsIFrame* aFrame, uint32_t aRenderedOffset,
                                              int32_t* aContentOffset) const
 {
@@ -2011,26 +2003,19 @@ HyperTextAccessible::RenderedToContentOf
   *aContentOffset = 0;
   NS_ENSURE_TRUE(aFrame, NS_ERROR_FAILURE);
 
   NS_ASSERTION(aFrame->GetType() == nsGkAtoms::textFrame,
                "Need text frame for offset conversion");
   NS_ASSERTION(aFrame->GetPrevContinuation() == nullptr,
                "Call on primary frame only");
 
-  gfxSkipChars skipChars;
-  gfxSkipCharsIterator iter;
-  // We only need info up to skipped offset -- that is what we're converting to original offset
-  nsresult rv = aFrame->GetRenderedText(nullptr, &skipChars, &iter, 0, aRenderedOffset);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  uint32_t ourRenderedStart = iter.GetSkippedOffset();
-  int32_t ourContentStart = iter.GetOriginalOffset();
-
-  *aContentOffset = iter.ConvertSkippedToOriginal(aRenderedOffset + ourRenderedStart) - ourContentStart;
+  nsIFrame::RenderedText text = aFrame->GetRenderedText(aRenderedOffset,
+      aRenderedOffset + 1, nsIFrame::TextOffsetType::OFFSETS_IN_RENDERED_TEXT);
+  *aContentOffset = text.mOffsetWithinNodeText;
 
   return NS_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // HyperTextAccessible public
 
 int32_t
--- a/accessible/tests/mochitest/jsat/test_traversal.html
+++ b/accessible/tests/mochitest/jsat/test_traversal.html
@@ -108,28 +108,28 @@
                               'Or me! ', 'Value 1', 'Value 2', 'Value 3',
                               'Electronic mailing address:', 'input-1-5',
                               'button-1-3', 'heading-2', 'heading-3',
                               'button-2-1', 'button-2-2', 'button-2-3',
                               'button-2-4', 'Programming Language',
                               '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',
                               'heading-5', 'image-2', 'image-3',
                               'Not actually an image', 'link-1', 'anchor-1',
                               'link-2', 'anchor-2', 'link-3', '3', '1', '4',
                               '1', 'Sunday', 'M', 'Week 1', '3', '4', '7', '2',
                               '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,
                              ['header-1', 'main-1', 'footer-1']);
 
 
       queueTraversalSequence(gQueue, docAcc, TraversalRules.Control, null,
                              ['input-1-1', 'label-1-2', 'button-1-1',
--- a/accessible/tests/mochitest/pivot/test_virtualcursor.html
+++ b/accessible/tests/mochitest/pivot/test_virtualcursor.html
@@ -47,17 +47,17 @@
 
       queueTraversalSequence(gQueue, docAcc, HeadersTraversalRule, null,
                              ['heading-1-1', 'heading-2-1', 'heading-2-2']);
 
       queueTraversalSequence(
         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']);
 
       // Just a random smoke test to see if our setTextRange works.
       gQueue.push(
         new setVCRangeInvoker(
           docAcc,
@@ -85,17 +85,17 @@
       // Attempting a coordinate outside any header, should move to null
       gQueue.push(new moveVCCoordInvoker(docAcc, x - 1, y - 1, false,
                                          HeadersTraversalRule, null));
 
       queueTraversalSequence(
         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));
 
       // Put cursor in an ignored subtree
       // set isFromUserInput to false, just to test..
       gQueue.push(new setVCPosInvoker(docAcc, null, null,
                                       getAccessible(doc.getElementById("hidden-link")),
--- a/accessible/tests/mochitest/text/test_doc.html
+++ b/accessible/tests/mochitest/text/test_doc.html
@@ -11,18 +11,18 @@
           src="../common.js"></script>
   <script type="application/javascript"
           src="../text.js"></script>
   <script type="application/javascript">
     
     function doTest()
     {
       var iframeDoc = [ getNode("iframe").contentDocument ];
-      testCharacterCount(iframeDoc, 15);
-      testText(iframeDoc, 0, 15, "outbody inbody ");
+      testCharacterCount(iframeDoc, 13);
+      testText(iframeDoc, 0, 13, "outbodyinbody");
 
       SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTest);
   </script>
 </head>
--- a/accessible/tests/mochitest/text/test_hypertext.html
+++ b/accessible/tests/mochitest/text/test_hypertext.html
@@ -53,17 +53,17 @@
       testText(IDs, 0, 1, "h");
       testText(IDs, 5, 7, " " + kEmbedChar);
       testText(IDs, 10, 13, "e " + kEmbedChar);
       testText(IDs, 0, 13, "hello " + kEmbedChar + " see " + kEmbedChar);
 
       ////////////////////////////////////////////////////////////////////////
       // getTextAtOffset line boundary
 
-      testTextAtOffset(0, BOUNDARY_LINE_START, "line ", 0, 5,
+      testTextAtOffset(0, BOUNDARY_LINE_START, "line", 0, 4,
                        "hypertext3", kOk, kOk, kOk);
 
       // XXX: see bug 634202.
       testTextAtOffset(0, BOUNDARY_LINE_START, "line ", 0, 5,
                        "hypertext4", kTodo, kOk, kTodo);
 
       //////////////////////////////////////////////////////////////////////////
       // list
@@ -117,17 +117,17 @@
 
   <p id="display"></p>
   <div id="content" style="display: none"></div>
   <pre id="test">
   </pre>
 
   <div id="nulltext"></div>
 
-  <div id="hypertext">hello <a>friend</a> see <img></div>
+  <div id="hypertext">hello <a>friend</a> see <img src="about:blank"></div>
   <div id="hypertext2">hello <a>friend</a> see <input></div>
   <ol id="list">
     <li id="listitem">foo</li>
     <li id="listitemnone">bar</li>
   </ol>
 
   <div id="hypertext3">line
 <!-- haha -->
--- a/accessible/tests/mochitest/text/test_lineboundary.html
+++ b/accessible/tests/mochitest/text/test_lineboundary.html
@@ -9,19 +9,19 @@
   <script type="application/javascript"
           src="../common.js"></script>
   <script type="application/javascript"
           src="../text.js"></script>
   <script type="application/javascript">
     function doTest()
     {
       testTextAtOffset("line_test_1", BOUNDARY_LINE_START,
-                       [[0, 6, "Line 1 ", 0, 7],
-                        [7, 7, "", 7, 7],
-                        [8, 15, "Line 3 ", 8, 15]]);
+                       [[0, 5, "Line 1", 0, 6],
+                        [6, 6, "", 6, 6],
+                        [7, 13, "Line 3", 7, 13]]);
 
       //////////////////////////////////////////////////////////////////////////
       // __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 = [ "input", "div", "editable", "textarea",
                   getNode("ta", getNode("ta_cntr").contentDocument) ];
 
@@ -109,20 +109,20 @@
       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 ] ]);
 
       //////////////////////////////////////////////////////////////////////////
-      // 'Hello world ' (\n is rendered as space)
+      // 'Hello world'
 
       testTextAtOffset([ "ht_4" ], BOUNDARY_LINE_START,
-                       [ [ 0, 12, "Hello world ", 0, 12 ] ]);
+                       [ [ 0, 11, "Hello world", 0, 11 ] ]);
 
       //////////////////////////////////////////////////////////////////////////
       // list items
 
       testTextAtOffset([ "li1" ], BOUNDARY_LINE_START,
                        [ [ 0, 6, kDiscBulletText + "Item", 0, 6 ] ]);
       testTextAtOffset([ "li2" ], BOUNDARY_LINE_START,
                        [ [ 0, 2, kDiscBulletText, 0, 2 ] ]);
--- a/accessible/tests/mochitest/text/test_wordboundary.html
+++ b/accessible/tests/mochitest/text/test_wordboundary.html
@@ -37,37 +37,30 @@
       // "hello "
       // __h__e__l__l__o__ __
       //  0  1  2  3  4  5  6
       var ids = [ "i2", "d2", "e2", "t2" ];
       testTextBeforeOffset(ids, BOUNDARY_WORD_START,
                            [ [ 0, 6, "", 0, 0 ] ]);
       testTextBeforeOffset(ids, BOUNDARY_WORD_END,
                            [ [ 0, 5, "", 0, 0 ],
-                             [ 6, 6, "hello", 0, 5,
-                               [ [6, "e2", kTodo, kOk, kTodo ] ]
-                             ]
+                             [ 6, 6, "hello", 0, 5 ]
                            ]);
 
       testTextAtOffset(ids, BOUNDARY_WORD_START,
                        [ [ 0, 6, "hello ", 0, 6 ] ]);
       testTextAtOffset(ids, BOUNDARY_WORD_END,
                        [ [ 0, 4, "hello", 0, 5 ],
-                         [ 5, 6, " ", 5, 6,
-                           [ [ 5, "e2", kTodo, kTodo, kOk ],
-                             [ 6, "e2", kTodo, kTodo, kOk ] ]
-                         ]
+                         [ 5, 6, " ", 5, 6 ]
                        ]);
 
       testTextAfterOffset(ids, BOUNDARY_WORD_START,
                           [ [ 0, 6, "", 6, 6 ] ]);
       testTextAfterOffset(ids, BOUNDARY_WORD_END,
-                          [ [ 0, 5, " ", 5, 6,
-                              [ [ 5, "e2", kTodo, kTodo, kOk ] ]
-                            ],
+                          [ [ 0, 5, " ", 5, 6 ],
                             [ 6, 6, "", 6, 6 ]
                           ]);
 
       // "hello all"
       // __h__e__l__l__o__ __a__l__l__
       //  0  1  2  3  4  5  6  7  8  9
       ids = [ "i6", "d6", "e6", "t6" ];
       testTextBeforeOffset(ids, BOUNDARY_WORD_START,
@@ -251,17 +244,17 @@
 
   <input id="i1" value="hello"/>
   <div id="d1">hello</div>
   <div id="e1" contenteditable="true">hello</div>
   <textarea id="t1">hello</textarea>
 
   <input id="i2" value="hello "/>
   <pre><div id="d2">hello </div></pre>
-  <div id="e2" contenteditable="true">hello </div>
+  <div id="e2" contenteditable="true" style='white-space:pre'>hello </div>
   <textarea id="t2">hello </textarea>
 
   <input id="i6" value="hello all"/>
   <div id="d6">hello all</div>
   <div id="e6" contenteditable="true">hello all</div>
   <textarea id="t6">hello all</textarea>
 
   <input id="i7" value="hello my friend"/>
--- a/accessible/tests/mochitest/tree/test_txtcntr.html
+++ b/accessible/tests/mochitest/tree/test_txtcntr.html
@@ -44,52 +44,52 @@
             role: ROLE_TEXT_LEAF,
             name: "Hello2"
           },
           {
             role: ROLE_SEPARATOR
           },
           {
             role: ROLE_TEXT_LEAF,
-            name: "Hello3 "
+            name: "Hello3"
           },
           {
             role: ROLE_PARAGRAPH,
             children: [
               {
                 role: ROLE_TEXT_LEAF,
-                name: "Hello4 "
+                name: "Hello4"
               }
             ]
           }
         ]
       };
 
       testAccessibleTree("c3", accTree);
 
       // contentEditable div
       accTree = {
         role: ROLE_SECTION,
         children: [
           {
             role: ROLE_TEXT_LEAF,
-            name: "helllo "
+            name: "helllo"
           },
           {
             role: ROLE_PARAGRAPH,
             children: [
               {
                 role: ROLE_TEXT_LEAF,
                 name: "blabla"
               }
             ]
           },
           {
             role: ROLE_TEXT_LEAF,
-            name: "hello "
+            name: "hello"
           }
         ]
       };
 
       testAccessibleTree("c4", accTree);
 
       // blockquote
       accTree = {
--- a/gfx/tests/gtest/TestSkipChars.cpp
+++ b/gfx/tests/gtest/TestSkipChars.cpp
@@ -77,17 +77,17 @@ TestIterator()
      9, 10, 11, 12, 13, 14, 15, 16, 17, 18,
     18, 19, 20, 21, 22, 23, 24, 25, 26, 27 };
 
   for (uint32_t i = 0; i < mozilla::ArrayLength(expectSkipped1); i++) {
     EXPECT_TRUE(iter1.ConvertOriginalToSkipped(i) == expectSkipped1[i]) <<
       "[4] Check mapping of original to skipped for " << i;
   }
 
-  uint32_t expectOriginal1[] =
+  int32_t expectOriginal1[] =
   {  0,  1,  2,  3,  4,  5,  6,  7,  8,
     10, 11, 12, 13, 14, 15, 16, 17, 18,
     20, 21, 22, 23, 24, 25, 26, 27, 28 };
 
   for (uint32_t i = 0; i < mozilla::ArrayLength(expectOriginal1); i++) {
     EXPECT_TRUE(iter1.ConvertSkippedToOriginal(i) == expectOriginal1[i]) <<
       "[5] Check mapping of skipped to original for " << i;
   }
@@ -131,17 +131,17 @@ TestIterator()
      1,  1,  1,  1,  1,  1,  1,  1,  1,  1,
      2,  2,  2,  2,  2,  2,  2,  2,  2,  2 };
 
   for (uint32_t i = 0; i < mozilla::ArrayLength(expectSkipped2); i++) {
     EXPECT_TRUE(iter2.ConvertOriginalToSkipped(i) == expectSkipped2[i]) <<
       "[9] Check mapping of original to skipped for " << i;
   }
 
-  uint32_t expectOriginal2[] = { 9, 19, 29 };
+  int32_t expectOriginal2[] = { 9, 19, 29 };
 
   for (uint32_t i = 0; i < mozilla::ArrayLength(expectOriginal2); i++) {
     EXPECT_TRUE(iter2.ConvertSkippedToOriginal(i) == expectOriginal2[i]) <<
       "[10] Check mapping of skipped to original for " << i;
   }
 
   bool expectIsOriginalSkipped2[] =
   {  true, true, true, true, true, true, true, true, true, false,
--- a/gfx/thebes/gfxSkipChars.h
+++ b/gfx/thebes/gfxSkipChars.h
@@ -230,17 +230,17 @@ public:
     void SetSkippedOffset(uint32_t aSkippedStringOffset);
 
     uint32_t ConvertOriginalToSkipped(int32_t aOriginalStringOffset)
     {
         SetOriginalOffset(aOriginalStringOffset);
         return GetSkippedOffset();
     }
 
-    uint32_t ConvertSkippedToOriginal(int32_t aSkippedStringOffset)
+    int32_t ConvertSkippedToOriginal(uint32_t aSkippedStringOffset)
     {
         SetSkippedOffset(aSkippedStringOffset);
         return GetOriginalOffset();
     }
 
     /**
      * Test if the character at the current position in the original string
      * is skipped or not. If aRunLength is non-null, then *aRunLength is set
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -1888,36 +1888,46 @@ public:
    * @return 
    *    true if we can continue a "text run" through the frame. A
    *    text run is text that should be treated contiguously for line
    *    and word breaking.
    */
   virtual bool CanContinueTextRun() const = 0;
 
   /**
-   * Append the rendered text to the passed-in string.
+   * Computes an approximation of the rendered text of the frame and its
+   * continuations. Returns nothing for non-text frames.
    * The appended text will often not contain all the whitespace from source,
-   * depending on whether the CSS rule "white-space: pre" is active for this frame.
-   * if aStartOffset + aLength goes past end, or if aLength is not specified
-   * then use the text up to the string's end.
+   * depending on CSS white-space processing.
+   * if aEndOffset goes past end, use the text up to the string's end.
    * Call this on the primary frame for a text node.
-   * @param aAppendToString   String to append text to, or null if text should not be returned
-   * @param aSkipChars         if aSkipIter is non-null, this must also be non-null.
-   * This gets used as backing data for the iterator so it should outlive the iterator.
-   * @param aSkipIter         Where to fill in the gfxSkipCharsIterator info, or null if not needed by caller
-   * @param aStartOffset       Skipped (rendered text) start offset
-   * @param aSkippedMaxLength  Maximum number of characters to return
-   * The iterator can be used to map content offsets to offsets in the returned string, or vice versa.
+   * aStartOffset and aEndOffset can be content offsets or offsets in the
+   * rendered text, depending on aOffsetType.
+   * Returns a string, as well as offsets identifying the start of the text
+   * within the rendered text for the whole node, and within the text content
+   * of the node.
    */
-  virtual nsresult GetRenderedText(nsAString* aAppendToString = nullptr,
-                                   gfxSkipChars* aSkipChars = nullptr,
-                                   gfxSkipCharsIterator* aSkipIter = nullptr,
-                                   uint32_t aSkippedStartOffset = 0,
-                                   uint32_t aSkippedMaxLength = UINT32_MAX)
-  { return NS_ERROR_NOT_IMPLEMENTED; }
+  struct RenderedText {
+    nsString mString;
+    uint32_t mOffsetWithinNodeRenderedText;
+    int32_t mOffsetWithinNodeText;
+    RenderedText() : mOffsetWithinNodeRenderedText(0),
+        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
+  };
+  virtual RenderedText GetRenderedText(uint32_t aStartOffset = 0,
+                                       uint32_t aEndOffset = UINT32_MAX,
+                                       TextOffsetType aOffsetType =
+                                           TextOffsetType::OFFSETS_IN_CONTENT_TEXT)
+  { 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()
   { return false; }
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -3970,19 +3970,18 @@ nsTextPaintStyle::GetResolvedForeColor(n
 
 //-----------------------------------------------------------------------------
 
 #ifdef ACCESSIBILITY
 a11y::AccType
 nsTextFrame::AccessibleType()
 {
   if (IsEmpty()) {
-    nsAutoString renderedWhitespace;
-    GetRenderedText(&renderedWhitespace, nullptr, nullptr, 0, 1);
-    if (renderedWhitespace.IsEmpty()) {
+    RenderedText text = GetRenderedText();
+    if (text.mString.IsEmpty()) {
       return a11y::eNoType;
     }
   }
 
   return a11y::eTextLeafType;
 }
 #endif
 
@@ -4085,23 +4084,16 @@ public:
   virtual nsIFrame* FirstInFlow() const override;
   virtual nsIFrame* FirstContinuation() const override;
 
   virtual void AddInlineMinISize(nsRenderingContext *aRenderingContext,
                                  InlineMinISizeData *aData) override;
   virtual void AddInlinePrefISize(nsRenderingContext *aRenderingContext,
                                   InlinePrefISizeData *aData) override;
   
-  virtual nsresult GetRenderedText(nsAString* aString = nullptr,
-                                   gfxSkipChars* aSkipChars = nullptr,
-                                   gfxSkipCharsIterator* aSkipIter = nullptr,
-                                   uint32_t aSkippedStartOffset = 0,
-                                   uint32_t aSkippedMaxLength = UINT32_MAX) override
-  { return NS_ERROR_NOT_IMPLEMENTED; } // Call on a primary text frame only
-
 protected:
   explicit nsContinuingTextFrame(nsStyleContext* aContext) : nsTextFrame(aContext) {}
   nsIFrame* mPrevContinuation;
 };
 
 void
 nsContinuingTextFrame::Init(nsIContent*       aContent,
                             nsContainerFrame* aParent,
@@ -8877,90 +8869,161 @@ static char16_t TransformChar(nsTextFram
       aChar = ToTitleCase(aChar);
     }
     break;
   }
 
   return aChar;
 }
 
-nsresult nsTextFrame::GetRenderedText(nsAString* aAppendToString,
-                                      gfxSkipChars* aSkipChars,
-                                      gfxSkipCharsIterator* aSkipIter,
-                                      uint32_t aSkippedStartOffset,
-                                      uint32_t aSkippedMaxLength)
-{
-  // The handling of aSkippedStartOffset and aSkippedMaxLength could be more efficient...
-  gfxSkipChars skipChars;
+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;
+  }
+  bool foundValidLine;
+  nsBlockInFlowLineIterator iter(block, aFrame, &foundValidLine);
+  if (!foundValidLine) {
+    NS_ERROR("Invalid line!");
+    return true;
+  }
+  return !iter.GetLine()->IsLineWrapped();
+}
+
+nsIFrame::RenderedText
+nsTextFrame::GetRenderedText(uint32_t aStartOffset,
+                             uint32_t aEndOffset,
+                             TextOffsetType aOffsetType)
+{
+  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 keptCharsLength = 0;
-  uint32_t validCharsLength = 0;
-
-  // Build skipChars and copy text, for each text frame in this continuation block
+  uint32_t offsetInRenderedString = 0;
+  bool haveOffsets = false;
+
   for (textFrame = this; textFrame;
        textFrame = static_cast<nsTextFrame*>(textFrame->GetNextContinuation())) {
-    // For each text frame continuation in this block ...
-
     if (textFrame->GetStateBits() & NS_FRAME_IS_DIRTY) {
-      // We don't trust dirty frames, expecially when computing rendered text.
+      // We don't trust dirty frames, especially when computing rendered text.
       break;
     }
 
     // Ensure the text run and grab the gfxSkipCharsIterator for it
     gfxSkipCharsIterator iter =
       textFrame->EnsureTextRun(nsTextFrame::eInflated);
-    if (!textFrame->mTextRun)
-      return NS_ERROR_FAILURE;
+    if (!textFrame->mTextRun) {
+      break;
+    }
+    gfxSkipCharsIterator tmpIter = iter;
 
     // Skip to the start of the text run, past ignored chars at start of line
-    // XXX In the future we may decide to trim extra spaces before a hard line
-    // break, in which case we need to accurately detect those sitations and 
-    // call GetTrimmedOffsets() with true to trim whitespace at the line's end
-    TrimmedOffsets trimmedContentOffsets = textFrame->GetTrimmedOffsets(textFrag, false);
-    int32_t startOfLineSkipChars = trimmedContentOffsets.mStart - textFrame->mContentOffset;
-    if (startOfLineSkipChars > 0) {
-      skipChars.SkipChars(startOfLineSkipChars);
-      iter.SetOriginalOffset(trimmedContentOffsets.mStart);
-    }
-
-    // Keep and copy the appropriate chars withing the caller's requested range
+    TrimmedOffsets trimmedOffsets = textFrame->GetTrimmedOffsets(textFrag,
+       textFrame->IsAtEndOfLine() && LineEndsInHardLineBreak(textFrame));
+    bool trimmedSignificantNewline =
+        trimmedOffsets.GetEnd() < GetContentEnd() &&
+        HasSignificantTerminalNewline();
+    uint32_t skippedToRenderedStringOffset = offsetInRenderedString -
+        tmpIter.ConvertOriginalToSkipped(trimmedOffsets.mStart);
+    uint32_t nextOffsetInRenderedString =
+        tmpIter.ConvertOriginalToSkipped(trimmedOffsets.GetEnd()) +
+        (trimmedSignificantNewline ? 1 : 0) + skippedToRenderedStringOffset;
+
+    if (aOffsetType == TextOffsetType::OFFSETS_IN_RENDERED_TEXT) {
+      if (nextOffsetInRenderedString <= aStartOffset) {
+        offsetInRenderedString = nextOffsetInRenderedString;
+        continue;
+      }
+      if (!haveOffsets) {
+        result.mOffsetWithinNodeText =
+            tmpIter.ConvertSkippedToOriginal(aStartOffset - skippedToRenderedStringOffset);
+        result.mOffsetWithinNodeRenderedText = aStartOffset;
+        haveOffsets = true;
+      }
+      if (offsetInRenderedString >= aEndOffset) {
+        break;
+      }
+    } else {
+      if (uint32_t(textFrame->GetContentEnd()) <= aStartOffset) {
+        offsetInRenderedString = nextOffsetInRenderedString;
+        continue;
+      }
+      if (!haveOffsets) {
+        result.mOffsetWithinNodeText = aStartOffset;
+        // Skip trimmed space when computed the rendered text offset.
+        int32_t clamped = std::max<int32_t>(aStartOffset, trimmedOffsets.mStart);
+        result.mOffsetWithinNodeRenderedText =
+            tmpIter.ConvertOriginalToSkipped(clamped) + skippedToRenderedStringOffset;
+        NS_ASSERTION(result.mOffsetWithinNodeRenderedText >= offsetInRenderedString &&
+                     result.mOffsetWithinNodeRenderedText <= INT32_MAX,
+                     "Bad offset within rendered text");
+        haveOffsets = true;
+      }
+      if (uint32_t(textFrame->mContentOffset) >= aEndOffset) {
+        break;
+      }
+    }
+
+    int32_t startOffset;
+    int32_t endOffset;
+    if (aOffsetType == TextOffsetType::OFFSETS_IN_RENDERED_TEXT) {
+      startOffset =
+        tmpIter.ConvertSkippedToOriginal(aStartOffset - skippedToRenderedStringOffset);
+      endOffset =
+        tmpIter.ConvertSkippedToOriginal(aEndOffset - skippedToRenderedStringOffset);
+    } else {
+      startOffset = aStartOffset;
+      endOffset = std::min<uint32_t>(INT32_MAX, aEndOffset);
+    }
+    trimmedOffsets.mStart = std::max<uint32_t>(trimmedOffsets.mStart,
+        startOffset);
+    trimmedOffsets.mLength = std::min<uint32_t>(trimmedOffsets.GetEnd(),
+        endOffset) - trimmedOffsets.mStart;
+    if (trimmedOffsets.mLength <= 0) {
+      offsetInRenderedString = nextOffsetInRenderedString;
+      continue;
+    }
+
     const nsStyleText* textStyle = textFrame->StyleText();
-    while (iter.GetOriginalOffset() < trimmedContentOffsets.GetEnd() &&
-           keptCharsLength < aSkippedMaxLength) {
-      // For each original char from content text
-      if (iter.IsOriginalCharSkipped() || ++validCharsLength <= aSkippedStartOffset) {
-        skipChars.SkipChar();
+    iter.SetOriginalOffset(trimmedOffsets.mStart);
+    while (iter.GetOriginalOffset() < trimmedOffsets.GetEnd()) {
+      char16_t ch = textFrag->CharAt(iter.GetOriginalOffset());
+      if (iter.IsOriginalCharSkipped()) {
+        if (ch == CH_SHY) {
+          // We should preserve soft hyphens. They can't be transformed.
+          result.mString.Append(ch);
+        }
       } else {
-        ++keptCharsLength;
-        skipChars.KeepChar();
-        if (aAppendToString) {
-          aAppendToString->Append(
+        result.mString.Append(
               TransformChar(textFrame, textStyle, textFrame->mTextRun,
-                            iter.GetSkippedOffset(),
-                            textFrag->CharAt(iter.GetOriginalOffset())));
-        }
+                            iter.GetSkippedOffset(), ch));
       }
       iter.AdvanceOriginal(1);
     }
-    if (keptCharsLength >= aSkippedMaxLength) {
-      break; // Already past the end, don't build string or gfxSkipCharsIter anymore
-    }
-  }
-  
-  if (aSkipChars) {
-    aSkipChars->TakeFrom(&skipChars); // Copy skipChars into aSkipChars
-    if (aSkipIter) {
-      // Caller must provide both pointers in order to retrieve a gfxSkipCharsIterator,
-      // because the gfxSkipCharsIterator holds a weak pointer to the gfxSkipChars.
-      *aSkipIter = gfxSkipCharsIterator(*aSkipChars, GetContentLength());
-    }
-  }
-
-  return NS_OK;
+
+    if (trimmedSignificantNewline && GetContentEnd() <= endOffset) {
+      // A significant newline was trimmed off (we must be
+      // white-space:pre-line). Put it back.
+      result.mString.Append('\n');
+    }
+    offsetInRenderedString = nextOffsetInRenderedString;
+  }
+
+  if (!haveOffsets) {
+    result.mOffsetWithinNodeText = textFrag->GetLength();
+    result.mOffsetWithinNodeRenderedText = offsetInRenderedString;
+  }
+  return result;
 }
 
 nsIAtom*
 nsTextFrame::GetType() const
 {
   return nsGkAtoms::textFrame;
 }
 
--- a/layout/generic/nsTextFrame.h
+++ b/layout/generic/nsTextFrame.h
@@ -256,21 +256,20 @@ public:
   struct TrimOutput {
     // true if we trimmed some space or changed metrics in some other way.
     // In this case, we should call RecomputeOverflow on this frame.
     bool mChanged;
     // an amount to *subtract* from the frame's width (zero if !mChanged)
     nscoord      mDeltaWidth;
   };
   TrimOutput TrimTrailingWhiteSpace(nsRenderingContext* aRC);
-  virtual nsresult GetRenderedText(nsAString* aString = nullptr,
-                                   gfxSkipChars* aSkipChars = nullptr,
-                                   gfxSkipCharsIterator* aSkipIter = nullptr,
-                                   uint32_t aSkippedStartOffset = 0,
-                                   uint32_t aSkippedMaxLength = UINT32_MAX) override;
+  virtual RenderedText GetRenderedText(uint32_t aStartOffset = 0,
+                                       uint32_t aEndOffset = UINT32_MAX,
+                                       TextOffsetType aOffsetType =
+                                           TextOffsetType::OFFSETS_IN_CONTENT_TEXT) 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,