Bug 152088

Summary: TextPainter: Rename start and end position to selectionStart and selectionEnd.
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, kondapallykalyan, mmaxfield, simon.fraser
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mmaxfield: review+

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>