RESOLVED FIXED 139715
Implement and adopt two new TextIndicator presentation animations 
https://bugs.webkit.org/show_bug.cgi?id=139715
Summary Implement and adopt two new TextIndicator presentation animations
Tim Horton
Reported 2014-12-16 17:55:37 PST
Implement and adopt two new TextIndicator presentation animations
Attachments
Patch (56.68 KB, patch)
2014-12-16 17:57 PST, Tim Horton 		no flags
DetailsFormatted DiffDiff
Patch (57.33 KB, patch)
2014-12-16 18:34 PST, Tim Horton 		andersca: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tim Horton
Comment 1 2014-12-16 17:57:28 PST
Created attachment 243413 [details] Patch
Tim Horton
Comment 2 2014-12-16 18:34:45 PST
Created attachment 243420 [details] Patch
Anders Carlsson
Comment 3 2014-12-16 18:35:36 PST
Comment on attachment 243413 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243413&action=review > Source/WebCore/page/mac/TextIndicatorWindow.mm:246 > + return _textIndicator->presentationTransition() == TextIndicatorPresentationTransition::BounceAndCrossfade || _textIndicator->presentationTransition() == TextIndicatorPresentationTransition::Bounce; > +} Please use a switch statement here instead. > Source/WebCore/page/mac/TextIndicatorWindow.mm:251 > + return (_textIndicator->presentationTransition() == TextIndicatorPresentationTransition::BounceAndCrossfade || _textIndicator->presentationTransition() == TextIndicatorPresentationTransition::Crossfade) && _textIndicator->contentImageWithHighlight(); > +} Please use a switch statement here instead. > Source/WebCore/page/mac/TextIndicatorWindow.mm:256 > + return _textIndicator->presentationTransition() == TextIndicatorPresentationTransition::FadeIn; > +} Please use a switch statement here instead. > Source/WebCore/page/mac/TextIndicatorWindow.mm:372 > + if (m_textIndicator->presentationTransition() == TextIndicatorPresentationTransition::Crossfade || m_textIndicator->presentationTransition() == TextIndicatorPresentationTransition::FadeIn) > + startFadeOut(); > + else > + closeWindow(); > +} Switch statement? > Source/WebCore/page/mac/TextIndicatorWindow.mm:412 > if (m_textIndicator->presentationTransition() != TextIndicatorPresentationTransition::None) { Single-line if statement.
Tim Horton
Comment 4 2014-12-16 19:02:31 PST
http://trac.webkit.org/changeset/177427
Note You need to log in before you can comment on or make changes to this bug.