⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 185804 in webkit


Ignore:
Timestamp:
Jun 20, 2015, 11:43:14 PM (11 years ago)
Author:
timothy_horton@apple.com
Message:

Deselection of text causes a noticeable jump on force touch machines
​https://bugs.webkit.org/show_bug.cgi?id=146173
<rdar://problem/20992842>

Reviewed by Sam Weinig.

Source/WebCore:

When we have a TextIndicator of type Crossfade, we end up putting
a layer with the blue highlight + text painted into it on top of the
content, and cross-fading that layer to the yellow-highlighted text.

This is necessary for BounceAndCrossfade TextIndicators, because the
blue highlight has to bounce, but is not necessary for Crossfade-only
ones; we can just fade in the yellow highlight on top of the
existing blue page highlight, and all is well.

So, get rid of the Crossfade TextIndicator type and use FadeIn, separately
keeping track of whether or not we can add a margin (we still can't
add a margin to TextIndicators that indicate the page's current selection,
because the blue highlight cannot have the margin applied to it, and we
want the bounds to match exactly).

  • page/TextIndicator.cpp:

(WebCore::TextIndicator::createWithRange):
If the range is the same as the selection, turn off the margin.
We were previously doing this based on the presentation transition, but now
there's no difference in presentation transition in this case.

(WebCore::TextIndicator::createWithSelectionInFrame):
(WebCore::TextIndicator::wantsBounce):
(WebCore::TextIndicator::wantsContentCrossfade):
(WebCore::TextIndicator::wantsFadeIn):
(WebCore::TextIndicator::wantsManualAnimation):

  • page/TextIndicator.h:

Get rid of TextIndicatorPresentationTransition::Crossfade.

(WebCore::TextIndicator::setWantsMargin):
(WebCore::TextIndicator::wantsMargin):
Keep track of whether we want a margin.

  • page/mac/TextIndicatorWindow.mm:

(-[WebTextIndicatorView initWithFrame:textIndicator:margin:]):
Determine if we should use a margin based on wantsMargin instead of the
presentation transition.

Source/WebKit/mac:

  • WebView/WebImmediateActionController.mm:

(-[WebImmediateActionController _animationControllerForText]):
Get rid of TextIndicatorPresentationTransition::Crossfade.

Source/WebKit2:

  • Shared/WebCoreArgumentCoders.cpp:

(IPC::ArgumentCoder<TextIndicatorData>::encode):
(IPC::ArgumentCoder<TextIndicatorData>::decode):
Encode/decode wantsMargin.

  • WebProcess/WebPage/mac/WebPageMac.mm:

(WebKit::WebPage::performImmediateActionHitTestAtLocation):
(WebKit::textIndicatorTransitionForImmediateAction): Deleted.
Get rid of TextIndicatorPresentationTransition::Crossfade.

Location:
trunk/Source
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r185790 r185804  
     12015-06-20  Tim Horton  <timothy_horton@apple.com>
     2
     3        Deselection of text causes a noticeable jump on force touch machines
     4        https://bugs.webkit.org/show_bug.cgi?id=146173
     5        <rdar://problem/20992842>
     6
     7        Reviewed by Sam Weinig.
     8
     9        When we have a TextIndicator of type Crossfade, we end up putting
     10        a layer with the blue highlight + text painted into it on top of the
     11        content, and cross-fading that layer to the yellow-highlighted text.
     12
     13        This is necessary for BounceAndCrossfade TextIndicators, because the
     14        blue highlight has to bounce, but is not necessary for Crossfade-only
     15        ones; we can just fade in the yellow highlight on top of the
     16        existing blue page highlight, and all is well.
     17
     18        So, get rid of the Crossfade TextIndicator type and use FadeIn, separately
     19        keeping track of whether or not we can add a margin (we still can't
     20        add a margin to TextIndicators that indicate the page's current selection,
     21        because the blue highlight cannot have the margin applied to it, and we
     22        want the bounds to match exactly).
     23
     24        * page/TextIndicator.cpp:
     25        (WebCore::TextIndicator::createWithRange):
     26        If the range is the same as the selection, turn off the margin.
     27        We were previously doing this based on the presentation transition, but now
     28        there's no difference in presentation transition in this case.
     29
     30        (WebCore::TextIndicator::createWithSelectionInFrame):
     31        (WebCore::TextIndicator::wantsBounce):
     32        (WebCore::TextIndicator::wantsContentCrossfade):
     33        (WebCore::TextIndicator::wantsFadeIn):
     34        (WebCore::TextIndicator::wantsManualAnimation):
     35        * page/TextIndicator.h:
     36        Get rid of TextIndicatorPresentationTransition::Crossfade.
     37
     38        (WebCore::TextIndicator::setWantsMargin):
     39        (WebCore::TextIndicator::wantsMargin):
     40        Keep track of whether we want a margin.
     41
     42        * page/mac/TextIndicatorWindow.mm:
     43        (-[WebTextIndicatorView initWithFrame:textIndicator:margin:]):
     44        Determine if we should use a margin based on wantsMargin instead of the
     45        presentation transition.
     46
    1472015-06-20  Ryuan Choi  <ryuan.choi@navercorp.com>
    248

  • trunk/Source/WebCore/page/TextIndicator.cpp

    r184066 r185804  
    3737#include "IntRect.h"
    3838#include "Page.h"
     39#include "Range.h"
    3940
    4041using namespace WebCore;
    … …  
    104105
    105106    frame->selection().setSelection(oldSelection);
     107
     108    indicator->setWantsMargin(!areRangesEqual(&range, oldSelection.toNormalizedRange().get()));
    106109   
    107110    return indicator.release();
    … …  
    142145
    143146    RefPtr<Image> indicatorBitmapWithHighlight;
    144     if (presentationTransition == TextIndicatorPresentationTransition::BounceAndCrossfade || presentationTransition == TextIndicatorPresentationTransition::Crossfade)
     147    if (presentationTransition == TextIndicatorPresentationTransition::BounceAndCrossfade)
    145148        indicatorBitmapWithHighlight = snapshotSelectionWithHighlight(frame);
    146149
    … …  
    177180    data.contentImageWithHighlight = indicatorBitmapWithHighlight;
    178181    data.presentationTransition = presentationTransition;
     182    data.wantsMargin = true;
    179183
    180184    return TextIndicator::create(data);
    … …  
    204208       
    205209    case TextIndicatorPresentationTransition::FadeIn:
    206     case TextIndicatorPresentationTransition::Crossfade:
    207210    case TextIndicatorPresentationTransition::None:
    208211        return false;
    … …  
    220223    switch (m_data.presentationTransition) {
    221224    case TextIndicatorPresentationTransition::BounceAndCrossfade:
    222     case TextIndicatorPresentationTransition::Crossfade:
    223225        return true;
    224226       
    … …  
    241243    case TextIndicatorPresentationTransition::Bounce:
    242244    case TextIndicatorPresentationTransition::BounceAndCrossfade:
    243     case TextIndicatorPresentationTransition::Crossfade:
    244245    case TextIndicatorPresentationTransition::None:
    245246        return false;
    … …  
    254255    switch (m_data.presentationTransition) {
    255256    case TextIndicatorPresentationTransition::FadeIn:
    256     case TextIndicatorPresentationTransition::Crossfade:
    257257        return true;
    258258

  • trunk/Source/WebCore/page/TextIndicator.h

    r184066 r185804  
    5252    BounceAndCrossfade,
    5353
    54     // These animations need to be driven manually via TextIndicatorWindow::setAnimationProgress.
     54    // This animation needs to be driven manually via TextIndicatorWindow::setAnimationProgress.
    5555    FadeIn,
    56     Crossfade
    5756};
    5857
    … …  
    7877    RefPtr<Image> contentImage;
    7978    TextIndicatorPresentationTransition presentationTransition;
     79    bool wantsMargin;
    8080};
    8181
    … …  
    105105    bool wantsManualAnimation() const;
    106106
     107    void setWantsMargin(bool wantsMargin) { m_data.wantsMargin = wantsMargin; }
     108    bool wantsMargin() const { return m_data.wantsMargin; }
     109
    107110private:
    108111    TextIndicator(const TextIndicatorData&);

  • trunk/Source/WebCore/page/mac/TextIndicatorWindow.mm

    r183870 r185804  
    103103    self.layer.anchorPoint = CGPointZero;
    104104
    105     bool wantsCrossfade = _textIndicator->wantsContentCrossfade();
    106 
    107105    FloatSize contentsImageLogicalSize = _textIndicator->contentImage()->size();
    108106    contentsImageLogicalSize.scale(1 / _textIndicator->contentImageScaleFactor());
    109107    RetainPtr<CGImageRef> contentsImage;
    110     if (wantsCrossfade)
     108    if (_textIndicator->wantsContentCrossfade())
    111109        contentsImage = _textIndicator->contentImageWithHighlight()->getCGImageRef();
    112110    else
    … …  
    138136        // FIXME (138888): Ideally we wouldn't remove the margin in this case, but we need to
    139137        // ensure that the yellow highlight and contentImageWithHighlight overlap precisely.
    140         if (wantsCrossfade) {
     138        if (!_textIndicator->wantsMargin()) {
    141139            yellowHighlightRect.inflateX(-horizontalBorder);
    142140            yellowHighlightRect.inflateY(-verticalBorder);

  • trunk/Source/WebKit/mac/ChangeLog

    r185766 r185804  
     12015-06-20  Tim Horton  <timothy_horton@apple.com>
     2
     3        Deselection of text causes a noticeable jump on force touch machines
     4        https://bugs.webkit.org/show_bug.cgi?id=146173
     5        <rdar://problem/20992842>
     6
     7        Reviewed by Sam Weinig.
     8
     9        * WebView/WebImmediateActionController.mm:
     10        (-[WebImmediateActionController _animationControllerForText]):
     11        Get rid of TextIndicatorPresentationTransition::Crossfade.
     12
    1132015-06-19  Brent Fulgham  <bfulgham@apple.com>
    214

  • trunk/Source/WebKit/mac/WebView/WebImmediateActionController.mm

    r183957 r185804  
    529529
    530530    RefPtr<Range> selectionRange = frame->page()->focusController().focusedOrMainFrame().selection().selection().firstRange();
    531     bool rangeMatchesSelection = areRangesEqual(dictionaryRange.get(), selectionRange.get());
    532     DictionaryPopupInfo dictionaryPopupInfo = dictionaryPopupInfoForRange(frame, *dictionaryRange, options, rangeMatchesSelection ? TextIndicatorPresentationTransition::Crossfade : TextIndicatorPresentationTransition::FadeIn);
     531    DictionaryPopupInfo dictionaryPopupInfo = dictionaryPopupInfoForRange(frame, *dictionaryRange, options, TextIndicatorPresentationTransition::FadeIn);
    533532    if (!dictionaryPopupInfo.attributedString)
    534533        return nil;

  • trunk/Source/WebKit2/ChangeLog

    r185803 r185804  
     12015-06-20  Tim Horton  <timothy_horton@apple.com>
     2
     3        Deselection of text causes a noticeable jump on force touch machines
     4        https://bugs.webkit.org/show_bug.cgi?id=146173
     5        <rdar://problem/20992842>
     6
     7        Reviewed by Sam Weinig.
     8
     9        * Shared/WebCoreArgumentCoders.cpp:
     10        (IPC::ArgumentCoder<TextIndicatorData>::encode):
     11        (IPC::ArgumentCoder<TextIndicatorData>::decode):
     12        Encode/decode wantsMargin.
     13
     14        * WebProcess/WebPage/mac/WebPageMac.mm:
     15        (WebKit::WebPage::performImmediateActionHitTestAtLocation):
     16        (WebKit::textIndicatorTransitionForImmediateAction): Deleted.
     17        Get rid of TextIndicatorPresentationTransition::Crossfade.
     18
    1192015-06-20  Dan Bernstein  <mitz@apple.com>
    220

  • trunk/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp

    r182869 r185804  
    21032103    encoder << textIndicatorData.textRectsInBoundingRectCoordinates;
    21042104    encoder << textIndicatorData.contentImageScaleFactor;
     2105    encoder << textIndicatorData.wantsMargin;
    21052106    encoder.encodeEnum(textIndicatorData.presentationTransition);
    21062107
    … …  
    21302131        return false;
    21312132
     2133    if (!decoder.decode(textIndicatorData.wantsMargin))
     2134        return false;
     2135
    21322136    if (!decoder.decodeEnum(textIndicatorData.presentationTransition))
    21332137        return false;

  • trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

    r184780 r185804  
    10631063}
    10641064
    1065 static TextIndicatorPresentationTransition textIndicatorTransitionForImmediateAction(Range* selectionRange, Range& indicatorRange, bool forDataDetectors)
    1066 {
    1067     if (areRangesEqual(&indicatorRange, selectionRange))
    1068         return TextIndicatorPresentationTransition::Crossfade;
    1069     return TextIndicatorPresentationTransition::FadeIn;
    1070 }
    1071 
    1072 #if ENABLE(PDFKIT_PLUGIN)
    1073 static TextIndicatorPresentationTransition textIndicatorTransitionForImmediateAction()
    1074 {
    1075     return TextIndicatorPresentationTransition::FadeIn;
    1076 }
    1077 #endif
    1078 
    10791065void WebPage::performImmediateActionHitTestAtLocation(WebCore::FloatPoint locationInViewCoordinates)
    10801066{
    … …  
    11051091    if (!absoluteLinkURL.isEmpty() && URLElement) {
    11061092        RefPtr<Range> linkRange = rangeOfContents(*URLElement);
    1107         immediateActionResult.linkTextIndicator = TextIndicator::createWithRange(*linkRange, textIndicatorTransitionForImmediateAction(selectionRange.get(), *linkRange, false));
     1093        immediateActionResult.linkTextIndicator = TextIndicator::createWithRange(*linkRange, TextIndicatorPresentationTransition::FadeIn);
    11081094    }
    11091095
    … …  
    11151101        if (Node* node = hitTestResult.innerNode()) {
    11161102            if (Frame* hitTestResultFrame = node->document().frame())
    1117                 immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForRange(hitTestResultFrame, *lookupRange.get(), &options, textIndicatorTransitionForImmediateAction(selectionRange.get(), *lookupRange, false));
     1103                immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForRange(hitTestResultFrame, *lookupRange.get(), &options, TextIndicatorPresentationTransition::FadeIn);
    11181104        }
    11191105    }
    … …  
    11411127
    11421128        immediateActionResult.detectedDataBoundingBox = detectedDataBoundingBox;
    1143         immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*mainResultRange, textIndicatorTransitionForImmediateAction(selectionRange.get(), *mainResultRange, true));
     1129        immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*mainResultRange, TextIndicatorPresentationTransition::FadeIn);
    11441130        immediateActionResult.detectedDataOriginatingPageOverlay = overlay->pageOverlayID();
    11451131
    … …  
    11541140        if (immediateActionResult.detectedDataActionContext && detectedDataRange) {
    11551141            immediateActionResult.detectedDataBoundingBox = detectedDataBoundingBox;
    1156             immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*detectedDataRange, textIndicatorTransitionForImmediateAction(selectionRange.get(), *detectedDataRange, true));
     1142            immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*detectedDataRange, TextIndicatorPresentationTransition::FadeIn);
    11571143        }
    11581144    }
    … …  
    11821168                immediateActionResult.allowsCopy = true;
    11831169
    1184                 immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForSelectionInPDFPlugin(selection, *pdfPugin, &options, textIndicatorTransitionForImmediateAction());
     1170                immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForSelectionInPDFPlugin(selection, *pdfPugin, &options, TextIndicatorPresentationTransition::FadeIn);
    11851171            }
    11861172        }
Note: See TracChangeset for help on using the changeset viewer.