144167 – Simple line layout: Wrong text offsetting when range does not start from the first renderer.

RESOLVED FIXED 144167

Simple line layout: Wrong text offsetting when range does not start from the first renderer.

https://bugs.webkit.org/show_bug.cgi?id=144167

Summary Simple line layout: Wrong text offsetting when range does not start from the ...

zalan

Reported 2015-04-24 15:52:31 PDT

<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.

Attachments
Patch (6.72 KB, patch) 2015-04-24 16:09 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (6.75 KB, patch) 2015-04-24 16:31 PDT, zalan	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-mavericks (734.52 KB, application/zip) 2015-04-24 16:49 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (719.67 KB, application/zip) 2015-04-24 17:04 PDT, Build Bot	no flags	Details
Patch (6.86 KB, patch) 2015-04-27 09:56 PDT, zalan	no flags	Details Formatted Diff Diff
Patch (6.84 KB, patch) 2015-04-27 11:04 PDT, zalan	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

zalan

Comment 1 2015-04-24 15:53:31 PDT

rdar://problem/20639857

zalan

Comment 2 2015-04-24 16:09:24 PDT

Created attachment 251583 [details] Patch

Simon Fraser (smfr)

Comment 3 2015-04-24 16:20:34 PDT

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.

zalan

Comment 4 2015-04-24 16:31:02 PDT

Created attachment 251588 [details] Patch

Simon Fraser (smfr)

Comment 5 2015-04-24 16:35:21 PDT

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>

Build Bot

Comment 6 2015-04-24 16:49:44 PDT

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

Build Bot

Comment 7 2015-04-24 16:49:47 PDT

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

Build Bot

Comment 8 2015-04-24 17:04:30 PDT

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

Build Bot

Comment 9 2015-04-24 17:04:34 PDT

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

zalan

Comment 10 2015-04-27 09:56:27 PDT

Created attachment 251748 [details] Patch

Darin Adler

Comment 11 2015-04-27 09:58:50 PDT

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?

zalan

Comment 12 2015-04-27 10:02:59 PDT

(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)

zalan

Comment 13 2015-04-27 11:04:46 PDT

Created attachment 251759 [details] Patch

WebKit Commit Bot

Comment 14 2015-04-27 13:20:27 PDT

Comment on attachment 251759 [details] Patch Clearing flags on attachment: 251759 Committed r183413: <http://trac.webkit.org/changeset/183413>

WebKit Commit Bot

Comment 15 2015-04-27 13:20:32 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component Layout and Rendering

Assignee

zalan

Reported

2015-04-24 15:52 PDT

Modified

2015-04-27 13:20 PDT History

CC List

5 users Show

URL

Keywords InRadar

Depends on

Blocks