<div>foobar1<br>foobar2<br>foobar3<br>foobar4<br>foobar5</div> 1. multiple renderers are present in the render flow. 2. Render flow uses simple line layout. 3. Range start node is not set to the first node in the flow. When text is called on Range, TextIterator returns the wrong selection.
rdar://problem/20639857
Created attachment 251583 [details] Patch
Comment on attachment 251583 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251583&action=review > Source/WebCore/editing/TextIterator.cpp:369 > + // Calculate the text offset for simple lines. > + Node* node = m_node->previousSibling(); > + while (node) { > + RenderObject* renderer = node->renderer(); > + if (renderer && is<RenderText>(renderer)) > + m_previousTextLengthInFlow += downcast<RenderText>(renderer)->textLength(); > + node = node->previousSibling(); > + } I think this should happen later when we know we're hitting a SLL code path.
Created attachment 251588 [details] Patch
Comment on attachment 251588 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251588&action=review > LayoutTests/fast/text/range-text-with-simple-line-layout.html:1 > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> <!DOCTYPE html>
Comment on attachment 251588 [details] Patch Attachment 251588 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5981947360706560 New failing tests: editing/deleting/delete-blockquote-large-offsets.html fast/spatial-navigation/snav-iframe-no-focusable-content.html editing/style/bold-with-dom-changes.html fast/parser/script-modify-page-outer.html fast/css/pseudo-empty-display-none.html editing/inserting/caret-position.html accessibility/aria-labelledby-with-descendants.html fast/files/file-reader-file-url.html fast/xmlhttprequest/xmlhttprequest-no-file-access.html fast/files/file-reader-sandbox-iframe.html
Created attachment 251593 [details] Archive of layout-test-results from ews102 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 251588 [details] Patch Attachment 251588 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5676253063413760 New failing tests: editing/deleting/delete-blockquote-large-offsets.html accessibility/aria-labelledby-with-descendants.html fast/parser/script-modify-page-outer.html fast/css/pseudo-empty-display-none.html fast/files/file-reader-sandbox-iframe.html fast/files/file-reader-file-url.html fast/xmlhttprequest/xmlhttprequest-no-file-access.html
Created attachment 251596 [details] Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Created attachment 251748 [details] Patch
Comment on attachment 251588 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251588&action=review > Source/WebCore/editing/TextIterator.cpp:509 > + Node* node = firstTextNodeInRange.previousSibling(); > + while (node) { Should be a for loop, not a while loop. > Source/WebCore/editing/TextIterator.cpp:512 > + if (renderer && is<RenderText>(renderer)) > + textOffset += downcast<RenderText>(renderer)->textLength(); What about a <br> or any other element that is treated as text by the text iterator? Just adding the literal text lengths of RenderText can’s be sufficient can it?
(In reply to comment #11) > Comment on attachment 251588 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=251588&action=review > > > Source/WebCore/editing/TextIterator.cpp:509 > > + Node* node = firstTextNodeInRange.previousSibling(); > > + while (node) { > > Should be a for loop, not a while loop. > Indeed. Thanks! > > Source/WebCore/editing/TextIterator.cpp:512 > > + if (renderer && is<RenderText>(renderer)) > > + textOffset += downcast<RenderText>(renderer)->textLength(); > > What about a <br> or any other element that is treated as text by the text > iterator? Just adding the literal text lengths of RenderText can’s be > sufficient can it? This is only used by simple line layout code path, so other text elements should not be included yet. (<br> does not have a length, only position)
Created attachment 251759 [details] Patch
Comment on attachment 251759 [details] Patch Clearing flags on attachment: 251759 Committed r183413: <http://trac.webkit.org/changeset/183413>
All reviewed patches have been landed. Closing bug.