Bug 152088 – TextPainter: Rename start and end position to selectionStart and selectionEnd.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

that's what they are.

[alan]

Created attachment 267035 [details]
Patch

[WebKit Commit Bot]

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.

[Simon Fraser (smfr)]

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?

[alan]

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

[alan]

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

[alan]

Committed r193857: <http://trac.webkit.org/changeset/193857>