Bug 138458 - TextIndicatorWindow's bounce animation is lopsided when indicating two short lines of text
Summary: TextIndicatorWindow's bounce animation is lopsided when indicating two short ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-11-06 01:33 PST by Tim Horton
Modified: 2014-11-06 11:39 PST (History)
5 users (show)

See Also:


Attachments
Patch (13.14 KB, patch)
2014-11-06 01:33 PST, Tim Horton
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2014-11-06 01:33:04 PST
TextIndicatorWindow's bounce animation is lopsided when indicating two short lines of text
Comment 1 Tim Horton 2014-11-06 01:33:29 PST
Created attachment 241095 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2014-11-06 01:35:27 PST
<rdar://problem/18892496>
Comment 3 Simon Fraser (smfr) 2014-11-06 11:04:38 PST
Comment on attachment 241095 [details]
Patch

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

> Source/WebKit2/Shared/TextIndicator.cpp:184
> +    for (const FloatRect& textRectInWindowCoordinates : textRectsInWindowCoordinates) {

auto? :\

I would be OK calling textRectInWindowCoordinates "rect".

> Source/WebKit2/Shared/TextIndicator.cpp:185
> +        FloatRect textRectInBoundingRectCoordinates = textRectInWindowCoordinates;

Why didn't you just get it by value?

> Source/WebKit2/Shared/TextIndicator.cpp:268
> +    for (size_t i = 0; i < m_data.textRectsInBoundingRectCoordinates.size(); ++i) {

Modern loop?

> Source/WebKit2/Shared/TextIndicator.cpp:322
> +            FloatRect imageRect = textRect;
> +            imageRect.move(m_data.textBoundingRectInWindowCoordinates.location() - m_data.selectionRectInWindowCoordinates.location());
> +            m_data.contentImage->paint(graphicsContext, m_data.contentImageScaleFactor, enclosingIntRect(textRect).location(), enclosingIntRect(imageRect));

I see this twice. Maybe use a helper function?
Comment 4 Tim Horton 2014-11-06 11:39:06 PST
http://trac.webkit.org/changeset/175705