Bug 138458

Summary: TextIndicatorWindow's bounce animation is lopsided when indicating two short lines of text
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, mitz, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch simon.fraser: review+

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