- Revision
- 185804
- Author
- [email protected]
- Date
- 2015-06-20 23:43:14 -0700 (Sat, 20 Jun 2015)
Log 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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (185803 => 185804)
--- trunk/Source/WebCore/ChangeLog 2015-06-21 03:27:20 UTC (rev 185803)
+++ trunk/Source/WebCore/ChangeLog 2015-06-21 06:43:14 UTC (rev 185804)
@@ -1,3 +1,49 @@
+2015-06-20 Tim Horton <[email protected]>
+
+ 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.
+
+ 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.
+
2015-06-20 Ryuan Choi <[email protected]>
[EFL] Do not consider test directories when DEVELOPER_MODE is OFF
Modified: trunk/Source/WebCore/page/TextIndicator.cpp (185803 => 185804)
--- trunk/Source/WebCore/page/TextIndicator.cpp 2015-06-21 03:27:20 UTC (rev 185803)
+++ trunk/Source/WebCore/page/TextIndicator.cpp 2015-06-21 06:43:14 UTC (rev 185804)
@@ -36,6 +36,7 @@
#include "ImageBuffer.h"
#include "IntRect.h"
#include "Page.h"
+#include "Range.h"
using namespace WebCore;
@@ -103,6 +104,8 @@
RefPtr<TextIndicator> indicator = TextIndicator::createWithSelectionInFrame(*frame, presentationTransition);
frame->selection().setSelection(oldSelection);
+
+ indicator->setWantsMargin(!areRangesEqual(&range, oldSelection.toNormalizedRange().get()));
return indicator.release();
}
@@ -141,7 +144,7 @@
return nullptr;
RefPtr<Image> indicatorBitmapWithHighlight;
- if (presentationTransition == TextIndicatorPresentationTransition::BounceAndCrossfade || presentationTransition == TextIndicatorPresentationTransition::Crossfade)
+ if (presentationTransition == TextIndicatorPresentationTransition::BounceAndCrossfade)
indicatorBitmapWithHighlight = snapshotSelectionWithHighlight(frame);
// Store the selection rect in window coordinates, to be used subsequently
@@ -176,6 +179,7 @@
data.contentImage = indicatorBitmap;
data.contentImageWithHighlight = indicatorBitmapWithHighlight;
data.presentationTransition = presentationTransition;
+ data.wantsMargin = true;
return TextIndicator::create(data);
}
@@ -203,7 +207,6 @@
return true;
case TextIndicatorPresentationTransition::FadeIn:
- case TextIndicatorPresentationTransition::Crossfade:
case TextIndicatorPresentationTransition::None:
return false;
}
@@ -219,7 +222,6 @@
switch (m_data.presentationTransition) {
case TextIndicatorPresentationTransition::BounceAndCrossfade:
- case TextIndicatorPresentationTransition::Crossfade:
return true;
case TextIndicatorPresentationTransition::Bounce:
@@ -240,7 +242,6 @@
case TextIndicatorPresentationTransition::Bounce:
case TextIndicatorPresentationTransition::BounceAndCrossfade:
- case TextIndicatorPresentationTransition::Crossfade:
case TextIndicatorPresentationTransition::None:
return false;
}
@@ -253,7 +254,6 @@
{
switch (m_data.presentationTransition) {
case TextIndicatorPresentationTransition::FadeIn:
- case TextIndicatorPresentationTransition::Crossfade:
return true;
case TextIndicatorPresentationTransition::Bounce:
Modified: trunk/Source/WebCore/page/TextIndicator.h (185803 => 185804)
--- trunk/Source/WebCore/page/TextIndicator.h 2015-06-21 03:27:20 UTC (rev 185803)
+++ trunk/Source/WebCore/page/TextIndicator.h 2015-06-21 06:43:14 UTC (rev 185804)
@@ -51,9 +51,8 @@
Bounce,
BounceAndCrossfade,
- // These animations need to be driven manually via TextIndicatorWindow::setAnimationProgress.
+ // This animation needs to be driven manually via TextIndicatorWindow::setAnimationProgress.
FadeIn,
- Crossfade
};
enum class TextIndicatorLifetime {
@@ -77,6 +76,7 @@
RefPtr<Image> contentImageWithHighlight;
RefPtr<Image> contentImage;
TextIndicatorPresentationTransition presentationTransition;
+ bool wantsMargin;
};
class TextIndicator : public RefCounted<TextIndicator> {
@@ -104,6 +104,9 @@
bool wantsFadeIn() const;
bool wantsManualAnimation() const;
+ void setWantsMargin(bool wantsMargin) { m_data.wantsMargin = wantsMargin; }
+ bool wantsMargin() const { return m_data.wantsMargin; }
+
private:
TextIndicator(const TextIndicatorData&);
Modified: trunk/Source/WebCore/page/mac/TextIndicatorWindow.mm (185803 => 185804)
--- trunk/Source/WebCore/page/mac/TextIndicatorWindow.mm 2015-06-21 03:27:20 UTC (rev 185803)
+++ trunk/Source/WebCore/page/mac/TextIndicatorWindow.mm 2015-06-21 06:43:14 UTC (rev 185804)
@@ -102,12 +102,10 @@
self.wantsLayer = YES;
self.layer.anchorPoint = CGPointZero;
- bool wantsCrossfade = _textIndicator->wantsContentCrossfade();
-
FloatSize contentsImageLogicalSize = _textIndicator->contentImage()->size();
contentsImageLogicalSize.scale(1 / _textIndicator->contentImageScaleFactor());
RetainPtr<CGImageRef> contentsImage;
- if (wantsCrossfade)
+ if (_textIndicator->wantsContentCrossfade())
contentsImage = _textIndicator->contentImageWithHighlight()->getCGImageRef();
else
contentsImage = _textIndicator->contentImage()->getCGImageRef();
@@ -137,7 +135,7 @@
FloatRect yellowHighlightRect(FloatPoint(), bounceLayerRect.size());
// FIXME (138888): Ideally we wouldn't remove the margin in this case, but we need to
// ensure that the yellow highlight and contentImageWithHighlight overlap precisely.
- if (wantsCrossfade) {
+ if (!_textIndicator->wantsMargin()) {
yellowHighlightRect.inflateX(-horizontalBorder);
yellowHighlightRect.inflateY(-verticalBorder);
}
Modified: trunk/Source/WebKit/mac/ChangeLog (185803 => 185804)
--- trunk/Source/WebKit/mac/ChangeLog 2015-06-21 03:27:20 UTC (rev 185803)
+++ trunk/Source/WebKit/mac/ChangeLog 2015-06-21 06:43:14 UTC (rev 185804)
@@ -1,3 +1,15 @@
+2015-06-20 Tim Horton <[email protected]>
+
+ 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.
+
+ * WebView/WebImmediateActionController.mm:
+ (-[WebImmediateActionController _animationControllerForText]):
+ Get rid of TextIndicatorPresentationTransition::Crossfade.
+
2015-06-19 Brent Fulgham <[email protected]>
All calls of ImageBuffer::create should null check the return value
Modified: trunk/Source/WebKit/mac/WebView/WebImmediateActionController.mm (185803 => 185804)
--- trunk/Source/WebKit/mac/WebView/WebImmediateActionController.mm 2015-06-21 03:27:20 UTC (rev 185803)
+++ trunk/Source/WebKit/mac/WebView/WebImmediateActionController.mm 2015-06-21 06:43:14 UTC (rev 185804)
@@ -528,8 +528,7 @@
return nil;
RefPtr<Range> selectionRange = frame->page()->focusController().focusedOrMainFrame().selection().selection().firstRange();
- bool rangeMatchesSelection = areRangesEqual(dictionaryRange.get(), selectionRange.get());
- DictionaryPopupInfo dictionaryPopupInfo = dictionaryPopupInfoForRange(frame, *dictionaryRange, options, rangeMatchesSelection ? TextIndicatorPresentationTransition::Crossfade : TextIndicatorPresentationTransition::FadeIn);
+ DictionaryPopupInfo dictionaryPopupInfo = dictionaryPopupInfoForRange(frame, *dictionaryRange, options, TextIndicatorPresentationTransition::FadeIn);
if (!dictionaryPopupInfo.attributedString)
return nil;
Modified: trunk/Source/WebKit2/ChangeLog (185803 => 185804)
--- trunk/Source/WebKit2/ChangeLog 2015-06-21 03:27:20 UTC (rev 185803)
+++ trunk/Source/WebKit2/ChangeLog 2015-06-21 06:43:14 UTC (rev 185804)
@@ -1,3 +1,21 @@
+2015-06-20 Tim Horton <[email protected]>
+
+ 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.
+
+ * 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.
+
2015-06-20 Dan Bernstein <[email protected]>
[Cocoa] Actually use Objective-C lightweight generics when the compiler supports them
Modified: trunk/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp (185803 => 185804)
--- trunk/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp 2015-06-21 03:27:20 UTC (rev 185803)
+++ trunk/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp 2015-06-21 06:43:14 UTC (rev 185804)
@@ -2102,6 +2102,7 @@
encoder << textIndicatorData.textBoundingRectInRootViewCoordinates;
encoder << textIndicatorData.textRectsInBoundingRectCoordinates;
encoder << textIndicatorData.contentImageScaleFactor;
+ encoder << textIndicatorData.wantsMargin;
encoder.encodeEnum(textIndicatorData.presentationTransition);
bool hasImage = textIndicatorData.contentImage;
@@ -2129,6 +2130,9 @@
if (!decoder.decode(textIndicatorData.contentImageScaleFactor))
return false;
+ if (!decoder.decode(textIndicatorData.wantsMargin))
+ return false;
+
if (!decoder.decodeEnum(textIndicatorData.presentationTransition))
return false;
Modified: trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm (185803 => 185804)
--- trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2015-06-21 03:27:20 UTC (rev 185803)
+++ trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm 2015-06-21 06:43:14 UTC (rev 185804)
@@ -1062,20 +1062,6 @@
return String();
}
-static TextIndicatorPresentationTransition textIndicatorTransitionForImmediateAction(Range* selectionRange, Range& indicatorRange, bool forDataDetectors)
-{
- if (areRangesEqual(&indicatorRange, selectionRange))
- return TextIndicatorPresentationTransition::Crossfade;
- return TextIndicatorPresentationTransition::FadeIn;
-}
-
-#if ENABLE(PDFKIT_PLUGIN)
-static TextIndicatorPresentationTransition textIndicatorTransitionForImmediateAction()
-{
- return TextIndicatorPresentationTransition::FadeIn;
-}
-#endif
-
void WebPage::performImmediateActionHitTestAtLocation(WebCore::FloatPoint locationInViewCoordinates)
{
layoutIfNeeded();
@@ -1104,7 +1090,7 @@
Element *URLElement = hitTestResult.URLElement();
if (!absoluteLinkURL.isEmpty() && URLElement) {
RefPtr<Range> linkRange = rangeOfContents(*URLElement);
- immediateActionResult.linkTextIndicator = TextIndicator::createWithRange(*linkRange, textIndicatorTransitionForImmediateAction(selectionRange.get(), *linkRange, false));
+ immediateActionResult.linkTextIndicator = TextIndicator::createWithRange(*linkRange, TextIndicatorPresentationTransition::FadeIn);
}
NSDictionary *options = nil;
@@ -1114,7 +1100,7 @@
if (lookupRange) {
if (Node* node = hitTestResult.innerNode()) {
if (Frame* hitTestResultFrame = node->document().frame())
- immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForRange(hitTestResultFrame, *lookupRange.get(), &options, textIndicatorTransitionForImmediateAction(selectionRange.get(), *lookupRange, false));
+ immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForRange(hitTestResultFrame, *lookupRange.get(), &options, TextIndicatorPresentationTransition::FadeIn);
}
}
@@ -1140,7 +1126,7 @@
detectedDataBoundingBox.unite(frameView->contentsToWindow(quad.enclosingBoundingBox()));
immediateActionResult.detectedDataBoundingBox = detectedDataBoundingBox;
- immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*mainResultRange, textIndicatorTransitionForImmediateAction(selectionRange.get(), *mainResultRange, true));
+ immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*mainResultRange, TextIndicatorPresentationTransition::FadeIn);
immediateActionResult.detectedDataOriginatingPageOverlay = overlay->pageOverlayID();
break;
@@ -1153,7 +1139,7 @@
immediateActionResult.detectedDataActionContext = DataDetection::detectItemAroundHitTestResult(hitTestResult, detectedDataBoundingBox, detectedDataRange);
if (immediateActionResult.detectedDataActionContext && detectedDataRange) {
immediateActionResult.detectedDataBoundingBox = detectedDataBoundingBox;
- immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*detectedDataRange, textIndicatorTransitionForImmediateAction(selectionRange.get(), *detectedDataRange, true));
+ immediateActionResult.detectedDataTextIndicator = TextIndicator::createWithRange(*detectedDataRange, TextIndicatorPresentationTransition::FadeIn);
}
}
@@ -1181,7 +1167,7 @@
immediateActionResult.isSelected = true;
immediateActionResult.allowsCopy = true;
- immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForSelectionInPDFPlugin(selection, *pdfPugin, &options, textIndicatorTransitionForImmediateAction());
+ immediateActionResult.dictionaryPopupInfo = dictionaryPopupInfoForSelectionInPDFPlugin(selection, *pdfPugin, &options, TextIndicatorPresentationTransition::FadeIn);
}
}
}
