Bug 152088 - TextPainter: Rename start and end position to selectionStart and selectionEnd.
Summary: TextPainter: Rename start and end position to selectionStart and selectionEnd.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-09 12:17 PST by zalan
Modified: 2015-12-09 13:22 PST (History)
6 users (show)

See Also:

Attachments
Patch (7.63 KB, patch)
2015-12-09 12:21 PST, zalan 		mmaxfield: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2015-12-09 12:17:35 PST 
that's what they are.
Comment 1 zalan 2015-12-09 12:21:40 PST 
Created attachment 267035 [details]
Patch
Comment 2 WebKit Commit Bot 2015-12-09 12:23:05 PST 
Attachment 267035 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/TextPainter.cpp:94:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebCore/rendering/TextPainter.cpp:95:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Simon Fraser (smfr) 2015-12-09 12:25:39 PST 
Comment on attachment 267035 [details]
Patch

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

> Source/WebCore/rendering/TextPainter.h:49
> +        int selectionStart, int selectionEnd, int length, const AtomicString& emphasisMark, RenderCombineText*,
> +        TextRun&, FloatRect& boxRect, FloatPoint& textOrigin, int emphasisMarkOffset, const ShadowData* textShadow, const ShadowData* selectionShadow,

I think 'selection' here is confusing. Is it really "selected" text, or just the start/end bits to paint?
Comment 4 zalan 2015-12-09 12:34:37 PST 
(In reply to comment #3)
> Comment on attachment 267035 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=267035&action=review
> 
> > Source/WebCore/rendering/TextPainter.h:49
> > +        int selectionStart, int selectionEnd, int length, const AtomicString& emphasisMark, RenderCombineText*,
> > +        TextRun&, FloatRect& boxRect, FloatPoint& textOrigin, int emphasisMarkOffset, const ShadowData* textShadow, const ShadowData* selectionShadow,
> 
> I think 'selection' here is confusing. Is it really "selected" text, or just
> the start/end bits to paint?
It is really the start and the end of the selected text. TextPainter::paintText() could be called with (!paintSelectedTextOnly and paintSelectedTextSeparately) which means we end up painting the non-selected text from 0-selectionStart and selectionEnd-length and then the selection run from selectionStart-selectionEnd.
Comment 5 zalan 2015-12-09 12:44:43 PST 
(In reply to comment #3)
> Comment on attachment 267035 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=267035&action=review
> 
> > Source/WebCore/rendering/TextPainter.h:49
> > +        int selectionStart, int selectionEnd, int length, const AtomicString& emphasisMark, RenderCombineText*,
> > +        TextRun&, FloatRect& boxRect, FloatPoint& textOrigin, int emphasisMarkOffset, const ShadowData* textShadow, const ShadowData* selectionShadow,
> 
> I think 'selection' here is confusing. Is it really "selected" text, or just
> the start/end bits to paint?
Also, 0/0 values are passed in when selection is not present(and we end up painting the run from 0-length)
Comment 6 zalan 2015-12-09 13:22:25 PST 
Committed r193857: <http://trac.webkit.org/changeset/193857>