Summary: | Implement and adopt two new TextIndicator presentation animations | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | andersca, bdakin, commit-queue, eric.carlson, glenn, jer.noble, philipj, sergio | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Tim Horton
2014-12-16 17:55:37 PST
Created attachment 243413 [details]
Patch
Created attachment 243420 [details]
Patch
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. |