Bug 148064 – Adopt shrink-wrapping for TextIndicators on Mac

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

Adopt shrink-wrapping for TextIndicators on Mac

[Tim Horton]

Created attachment 259113 [details]
Patch

[Tim Horton]

Created attachment 259114 [details]
before and after screenshots

[Beth Dakin]

Comment on attachment 259113 [details]
Patch

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

> Source/WebCore/page/TextIndicator.cpp:85
> +    bool indicatesCurrentSelection = areRangesEqual(&range, oldSelection.toNormalizedRange().get());

I just want to double check that this is doing the right thing. The code that you removed set data.indicatesCurrentSelection to !areRangesEqual(…), but you are setting a variable of the same name to the logical opposite. Just making sure that is intentional!

[Tim Horton]

(In reply to comment #3)
> Comment on attachment 259113 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=259113&action=review
> 
> > Source/WebCore/page/TextIndicator.cpp:85
> > +    bool indicatesCurrentSelection = areRangesEqual(&range, oldSelection.toNormalizedRange().get());
> 
> I just want to double check that this is doing the right thing. The code
> that you removed set data.indicatesCurrentSelection to !areRangesEqual(…),
> but you are setting a variable of the same name to the logical opposite.
> Just making sure that is intentional!

Yes, that was an error in the most recent previous TextIndicator patch. If the ranges are equal (the incoming range and the selection range), then we are indicating the selection, so no negation necessary. Thanks for the careful review!

[Tim Horton]

http://trac.webkit.org/changeset/188526