Bug 178232 – Share logic in InlineTextBox to compute selection rect

• Status: REOPENED
• Component: Layout and Rendering

[Daniel Bates]

Currently each paint routine in InlineTextBox computes the selection rect it will paint using the same algorithm or an algorithm that varies slightly. The slight variations represent simplifications of the algorithm that are specific to what is being painted. For example, InlineTextBox::paintDocumentMarker() takes advantage of the fact that the marker decoration is an underline such that it does not concern itself with overlapping backgrounds as InlineTextBox::paintSelection() needs to do. Therefore, InlineTextBox::paintDocumentMarker() can assume the height of the marker is the height of selected text in the box ignoring overlap (call InlineTextBox::selectionHeight()). In contrast, InlineTextBox::paintSelection() must compute the height taking into account the height of any selected text in the last line of the proceeding block (using RootInlineBox::selectionTopAdjustedForPrecedingBlock()) so as make the illusion that the background for all of the selected text on the page was painted in one continuous paint operation as opposed to being painted piecewise. We should share more code.

[Radar WebKit Bug Importer]

<rdar://problem/34963452>

[Daniel Bates]

Created attachment 323564 [details]
Patch

[Daniel Bates]

Created attachment 323566 [details]
Patch

[Daniel Bates]

Created attachment 323575 [details]
Patch

[Daniel Bates]

Comment on attachment 323575 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=323575&action=review

> Source/WebCore/rendering/InlineTextBox.cpp:677
> +    context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), direction() == LTR), c);

Will substitute isLeftToRightDirection() for "direction() == LTR" before landing.

> Source/WebCore/rendering/InlineTextBox.cpp:699
> +    context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), direction() == LTR), color);

Ditto.

[zalan]

Comment on attachment 323575 [details]
Patch

This patch could use a bit of 'auto'.

[Daniel Bates]

Committed r223699: <https://trac.webkit.org/changeset/223699>

[Daniel Bates]

Reverted r223699 for reason:

Caused regressions with right-to-left text selection and painting of markers in flipped writing mode and in overlapping lines. Will investigate offline.

Committed r223840: <https://trac.webkit.org/changeset/223840>

[Daniel Bates]

Created attachment 324568 [details]
Work-in-progress

Incorporates the source code change from the patch for bug #178611, attachment #324477 [details].

[Build Bot]

Comment on attachment 324568 [details]
Work-in-progress

Attachment 324568 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4960534

New failing tests:
platform/mac/fast/dom/character-index-for-point.html
fast/repaint/selection-ruby-rl.html
editing/spelling/spelling-markers-in-overlapping-lines.html
editing/spelling/spelling-markers-in-overlapping-lines-large-font.html
fast/dom/Range/getClientRects.html

[Build Bot]

Created attachment 324574 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 324568 [details]
Work-in-progress

Attachment 324568 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4960488

New failing tests:
platform/mac/fast/dom/character-index-for-point.html
fast/repaint/selection-ruby-rl.html
editing/spelling/spelling-markers-in-overlapping-lines.html
fast/dom/Range/getClientRects.html
editing/spelling/spelling-markers-in-overlapping-lines-large-font.html

[Build Bot]

Created attachment 324575 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 324568 [details]
Work-in-progress

Attachment 324568 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4964592

New failing tests:
fast/repaint/selection-ruby-rl.html
fast/dom/Range/getClientRects.html

[Build Bot]

Created attachment 324625 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6