| Summary: | Adopt shrink-wrapping for TextIndicators on Mac | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
| Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | andersca, bdakin, enrica, mitz, sam, simon.fraser | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Tim Horton
2015-08-15 23:28:54 PDT
Created attachment 259113 [details]
Patch
Created attachment 259114 [details]
before and after screenshots
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! (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! |