- Revision
- 178290
- Author
- timothy_hor...@apple.com
- Date
- 2015-01-12 12:22:10 -0800 (Mon, 12 Jan 2015)
Log Message
Multi-rect TextIndicators are vertically flipped in WebKit1
https://bugs.webkit.org/show_bug.cgi?id=140350
<rdar://problem/19441243>
Reviewed by Beth Dakin.
* page/TextIndicator.cpp:
(WebCore::TextIndicator::createWithSelectionInFrame):
(WebCore::TextIndicator::TextIndicator):
* page/TextIndicator.h:
(WebCore::TextIndicator::selectionRectInRootViewCoordinates):
(WebCore::TextIndicator::textBoundingRectInRootViewCoordinates):
(WebCore::TextIndicator::selectionRectInWindowCoordinates): Deleted.
(WebCore::TextIndicator::textBoundingRectInWindowCoordinates): Deleted.
* page/mac/TextIndicatorWindow.mm:
(-[WebTextIndicatorView initWithFrame:textIndicator:margin:]):
(WebCore::TextIndicatorWindow::setTextIndicator):
Compute, store, and use TextIndicator's selectionRect and textBoundingRect
in root view coordinates instead of window coordinates; this way, each
WebKit can do the conversion itself, and the rootView vs. window flipping
isn't wrongly factored into textRectsInBoundingRectCoordinates.
* Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<TextIndicatorData>::encode):
(IPC::ArgumentCoder<TextIndicatorData>::decode):
Adjust to the field name changes.
* UIProcess/API/mac/WKView.mm:
(-[WKView _setTextIndicator:fadeOut:]):
Convert the textBoundingRect from root view to screen coordinates.
* WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::updateFindIndicator):
(WebKit::FindController::drawRect):
Adjust to the new name, and use contentsToRootView when comparing against
the stored m_findIndicatorRect.
* WebView/WebView.mm:
(-[WebView _setTextIndicator:fadeOut:]):
Convert the textBoundingRect from root view to screen coordinates.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (178289 => 178290)
--- trunk/Source/WebCore/ChangeLog 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebCore/ChangeLog 2015-01-12 20:22:10 UTC (rev 178290)
@@ -1,3 +1,27 @@
+2015-01-12 Timothy Horton <timothy_hor...@apple.com>
+
+ Multi-rect TextIndicators are vertically flipped in WebKit1
+ https://bugs.webkit.org/show_bug.cgi?id=140350
+ <rdar://problem/19441243>
+
+ Reviewed by Beth Dakin.
+
+ * page/TextIndicator.cpp:
+ (WebCore::TextIndicator::createWithSelectionInFrame):
+ (WebCore::TextIndicator::TextIndicator):
+ * page/TextIndicator.h:
+ (WebCore::TextIndicator::selectionRectInRootViewCoordinates):
+ (WebCore::TextIndicator::textBoundingRectInRootViewCoordinates):
+ (WebCore::TextIndicator::selectionRectInWindowCoordinates): Deleted.
+ (WebCore::TextIndicator::textBoundingRectInWindowCoordinates): Deleted.
+ * page/mac/TextIndicatorWindow.mm:
+ (-[WebTextIndicatorView initWithFrame:textIndicator:margin:]):
+ (WebCore::TextIndicatorWindow::setTextIndicator):
+ Compute, store, and use TextIndicator's selectionRect and textBoundingRect
+ in root view coordinates instead of window coordinates; this way, each
+ WebKit can do the conversion itself, and the rootView vs. window flipping
+ isn't wrongly factored into textRectsInBoundingRectCoordinates.
+
2015-01-12 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r178281.
Modified: trunk/Source/WebCore/page/TextIndicator.cpp (178289 => 178290)
--- trunk/Source/WebCore/page/TextIndicator.cpp 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebCore/page/TextIndicator.cpp 2015-01-12 20:22:10 UTC (rev 178290)
@@ -146,7 +146,7 @@
// Store the selection rect in window coordinates, to be used subsequently
// to determine if the indicator and selection still precisely overlap.
- IntRect selectionRectInWindowCoordinates = frame.view()->contentsToWindow(selectionRect);
+ IntRect selectionRectInRootViewCoordinates = frame.view()->contentsToRootView(selectionRect);
Vector<FloatRect> textRects;
frame.selection().getClippedVisibleTextRectangles(textRects);
@@ -154,23 +154,23 @@
// The bounding rect of all the text rects can be different than the selection
// rect when the selection spans multiple lines; the indicator doesn't actually
// care where the selection highlight goes, just where the text actually is.
- FloatRect textBoundingRectInWindowCoordinates;
- Vector<FloatRect> textRectsInWindowCoordinates;
+ FloatRect textBoundingRectInRootViewCoordinates;
+ Vector<FloatRect> textRectsInRootViewCoordinates;
for (const FloatRect& textRect : textRects) {
- FloatRect textRectInWindowCoordinates = frame.view()->contentsToWindow(enclosingIntRect(textRect));
- textRectsInWindowCoordinates.append(textRectInWindowCoordinates);
- textBoundingRectInWindowCoordinates.unite(textRectInWindowCoordinates);
+ FloatRect textRectInRootViewCoordinates = frame.view()->contentsToRootView(enclosingIntRect(textRect));
+ textRectsInRootViewCoordinates.append(textRectInRootViewCoordinates);
+ textBoundingRectInRootViewCoordinates.unite(textRectInRootViewCoordinates);
}
Vector<FloatRect> textRectsInBoundingRectCoordinates;
- for (auto rect : textRectsInWindowCoordinates) {
- rect.moveBy(-textBoundingRectInWindowCoordinates.location());
+ for (auto rect : textRectsInRootViewCoordinates) {
+ rect.moveBy(-textBoundingRectInRootViewCoordinates.location());
textRectsInBoundingRectCoordinates.append(rect);
}
TextIndicatorData data;
- data.selectionRectInWindowCoordinates = selectionRectInWindowCoordinates;
- data.textBoundingRectInWindowCoordinates = textBoundingRectInWindowCoordinates;
+ data.selectionRectInRootViewCoordinates = selectionRectInRootViewCoordinates;
+ data.textBoundingRectInRootViewCoordinates = textBoundingRectInRootViewCoordinates;
data.textRectsInBoundingRectCoordinates = textRectsInBoundingRectCoordinates;
data.contentImageScaleFactor = frame.page()->deviceScaleFactor();
data.contentImage = indicatorBitmap;
@@ -183,7 +183,7 @@
TextIndicator::TextIndicator(const TextIndicatorData& data)
: m_data(data)
{
- ASSERT(m_data.contentImageScaleFactor != 1 || m_data.contentImage->size() == enclosingIntRect(m_data.selectionRectInWindowCoordinates).size());
+ ASSERT(m_data.contentImageScaleFactor != 1 || m_data.contentImage->size() == enclosingIntRect(m_data.selectionRectInRootViewCoordinates).size());
if (textIndicatorsForTextRectsOverlap(m_data.textRectsInBoundingRectCoordinates)) {
m_data.textRectsInBoundingRectCoordinates[0] = unionRect(m_data.textRectsInBoundingRectCoordinates);
Modified: trunk/Source/WebCore/page/TextIndicator.h (178289 => 178290)
--- trunk/Source/WebCore/page/TextIndicator.h 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebCore/page/TextIndicator.h 2015-01-12 20:22:10 UTC (rev 178290)
@@ -57,8 +57,8 @@
};
struct TextIndicatorData {
- FloatRect selectionRectInWindowCoordinates;
- FloatRect textBoundingRectInWindowCoordinates;
+ FloatRect selectionRectInRootViewCoordinates;
+ FloatRect textBoundingRectInRootViewCoordinates;
Vector<FloatRect> textRectsInBoundingRectCoordinates;
float contentImageScaleFactor;
RefPtr<Image> contentImageWithHighlight;
@@ -74,8 +74,8 @@
WEBCORE_EXPORT ~TextIndicator();
- FloatRect selectionRectInWindowCoordinates() const { return m_data.selectionRectInWindowCoordinates; }
- FloatRect textBoundingRectInWindowCoordinates() const { return m_data.textBoundingRectInWindowCoordinates; }
+ FloatRect selectionRectInRootViewCoordinates() const { return m_data.selectionRectInRootViewCoordinates; }
+ FloatRect textBoundingRectInRootViewCoordinates() const { return m_data.textBoundingRectInRootViewCoordinates; }
const Vector<FloatRect>& textRectsInBoundingRectCoordinates() const { return m_data.textRectsInBoundingRectCoordinates; }
float contentImageScaleFactor() const { return m_data.contentImageScaleFactor; }
Image *contentImageWithHighlight() const { return m_data.contentImageWithHighlight.get(); }
Modified: trunk/Source/WebCore/page/mac/TextIndicatorWindow.mm (178289 => 178290)
--- trunk/Source/WebCore/page/mac/TextIndicatorWindow.mm 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebCore/page/mac/TextIndicatorWindow.mm 2015-01-12 20:22:10 UTC (rev 178290)
@@ -116,7 +116,7 @@
RetainPtr<CGColorRef> gradientDarkColor = [NSColor colorWithDeviceRed:.929 green:.8 blue:0 alpha:1].CGColor;
RetainPtr<CGColorRef> gradientLightColor = [NSColor colorWithDeviceRed:.949 green:.937 blue:0 alpha:1].CGColor;
- for (auto& textRect : _textIndicator->textRectsInBoundingRectCoordinates()) {
+ for (const auto& textRect : _textIndicator->textRectsInBoundingRectCoordinates()) {
FloatRect bounceLayerRect = textRect;
bounceLayerRect.move(_margin.width, _margin.height);
bounceLayerRect.inflateX(horizontalBorder);
@@ -176,7 +176,7 @@
[textLayer setContents:(id)contentsImage.get()];
FloatRect imageRect = textRect;
- imageRect.move(_textIndicator->textBoundingRectInWindowCoordinates().location() - _textIndicator->selectionRectInWindowCoordinates().location());
+ imageRect.move(_textIndicator->textBoundingRectInRootViewCoordinates().location() - _textIndicator->selectionRectInRootViewCoordinates().location());
[textLayer setContentsRect:CGRectMake(imageRect.x() / contentsImageLogicalSize.width(), imageRect.y() / contentsImageLogicalSize.height(), imageRect.width() / contentsImageLogicalSize.width(), imageRect.height() / contentsImageLogicalSize.height())];
[textLayer setContentsGravity:kCAGravityCenter];
[textLayer setContentsScale:_textIndicator->contentImageScaleFactor()];
@@ -369,14 +369,13 @@
[m_textIndicatorView setAnimationProgress:progress];
}
-void TextIndicatorWindow::setTextIndicator(PassRefPtr<TextIndicator> textIndicator, CGRect contentRect, bool fadeOut)
+void TextIndicatorWindow::setTextIndicator(PassRefPtr<TextIndicator> textIndicator, CGRect textBoundingRectInScreenCoordinates, bool fadeOut)
{
if (m_textIndicator == textIndicator)
return;
m_textIndicator = textIndicator;
- // Get rid of the old window.
closeWindow();
if (!m_textIndicator)
@@ -386,11 +385,11 @@
CGFloat verticalMargin = dropShadowBlurRadius * 2 + verticalBorder;
if (m_textIndicator->wantsBounce()) {
- horizontalMargin = std::max(horizontalMargin, contentRect.size.width * (midBounceScale - 1) + horizontalMargin);
- verticalMargin = std::max(verticalMargin, contentRect.size.height * (midBounceScale - 1) + verticalMargin);
+ horizontalMargin = std::max(horizontalMargin, textBoundingRectInScreenCoordinates.size.width * (midBounceScale - 1) + horizontalMargin);
+ verticalMargin = std::max(verticalMargin, textBoundingRectInScreenCoordinates.size.height * (midBounceScale - 1) + verticalMargin);
}
- contentRect = CGRectInset(contentRect, -horizontalMargin, -verticalMargin);
+ CGRect contentRect = CGRectInset(textBoundingRectInScreenCoordinates, -horizontalMargin, -verticalMargin);
NSRect windowContentRect = [NSWindow contentRectForFrameRect:NSIntegralRect(NSRectFromCGRect(contentRect)) styleMask:NSBorderlessWindowMask];
m_textIndicatorWindow = adoptNS([[NSWindow alloc] initWithContentRect:windowContentRect styleMask:NSBorderlessWindowMask backing:NSBackingStoreBuffered defer:NO]);
Modified: trunk/Source/WebKit/mac/ChangeLog (178289 => 178290)
--- trunk/Source/WebKit/mac/ChangeLog 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebKit/mac/ChangeLog 2015-01-12 20:22:10 UTC (rev 178290)
@@ -1,3 +1,15 @@
+2015-01-12 Timothy Horton <timothy_hor...@apple.com>
+
+ Multi-rect TextIndicators are vertically flipped in WebKit1
+ https://bugs.webkit.org/show_bug.cgi?id=140350
+ <rdar://problem/19441243>
+
+ Reviewed by Beth Dakin.
+
+ * WebView/WebView.mm:
+ (-[WebView _setTextIndicator:fadeOut:]):
+ Convert the textBoundingRect from root view to screen coordinates.
+
2015-01-12 Anders Carlsson <ander...@apple.com>
Make WebResourceDelegate a formal protocol as well
Modified: trunk/Source/WebKit/mac/WebView/WebView.mm (178289 => 178290)
--- trunk/Source/WebKit/mac/WebView/WebView.mm 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebKit/mac/WebView/WebView.mm 2015-01-12 20:22:10 UTC (rev 178290)
@@ -8653,8 +8653,9 @@
if (!_private->textIndicatorWindow)
_private->textIndicatorWindow = std::make_unique<TextIndicatorWindow>(self);
- NSRect contentRect = [self.window convertRectToScreen:textIndicator->textBoundingRectInWindowCoordinates()];
- _private->textIndicatorWindow->setTextIndicator(textIndicator, NSRectToCGRect(contentRect), fadeOut);
+ NSRect textBoundingRectInWindowCoordinates = [self convertRect:[self _convertRectFromRootView:textIndicator->textBoundingRectInRootViewCoordinates()] toView:nil];
+ NSRect textBoundingRectInScreenCoordinates = [self.window convertRectToScreen:textBoundingRectInWindowCoordinates];
+ _private->textIndicatorWindow->setTextIndicator(textIndicator, NSRectToCGRect(textBoundingRectInScreenCoordinates), fadeOut);
}
- (void)_clearTextIndicator
Modified: trunk/Source/WebKit2/ChangeLog (178289 => 178290)
--- trunk/Source/WebKit2/ChangeLog 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebKit2/ChangeLog 2015-01-12 20:22:10 UTC (rev 178290)
@@ -1,3 +1,26 @@
+2015-01-12 Timothy Horton <timothy_hor...@apple.com>
+
+ Multi-rect TextIndicators are vertically flipped in WebKit1
+ https://bugs.webkit.org/show_bug.cgi?id=140350
+ <rdar://problem/19441243>
+
+ Reviewed by Beth Dakin.
+
+ * Shared/WebCoreArgumentCoders.cpp:
+ (IPC::ArgumentCoder<TextIndicatorData>::encode):
+ (IPC::ArgumentCoder<TextIndicatorData>::decode):
+ Adjust to the field name changes.
+
+ * UIProcess/API/mac/WKView.mm:
+ (-[WKView _setTextIndicator:fadeOut:]):
+ Convert the textBoundingRect from root view to screen coordinates.
+
+ * WebProcess/WebPage/FindController.cpp:
+ (WebKit::FindController::updateFindIndicator):
+ (WebKit::FindController::drawRect):
+ Adjust to the new name, and use contentsToRootView when comparing against
+ the stored m_findIndicatorRect.
+
2015-01-11 Ryuan Choi <ryuan.c...@navercorp.com>
[CoordinatedGraphics] Suspend or resume when visibility is changed
Modified: trunk/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp (178289 => 178290)
--- trunk/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp 2015-01-12 20:22:10 UTC (rev 178290)
@@ -1963,8 +1963,8 @@
void ArgumentCoder<TextIndicatorData>::encode(ArgumentEncoder& encoder, const TextIndicatorData& textIndicatorData)
{
- encoder << textIndicatorData.selectionRectInWindowCoordinates;
- encoder << textIndicatorData.textBoundingRectInWindowCoordinates;
+ encoder << textIndicatorData.selectionRectInRootViewCoordinates;
+ encoder << textIndicatorData.textBoundingRectInRootViewCoordinates;
encoder << textIndicatorData.textRectsInBoundingRectCoordinates;
encoder << textIndicatorData.contentImageScaleFactor;
encoder.encodeEnum(textIndicatorData.presentationTransition);
@@ -1982,10 +1982,10 @@
bool ArgumentCoder<TextIndicatorData>::decode(ArgumentDecoder& decoder, TextIndicatorData& textIndicatorData)
{
- if (!decoder.decode(textIndicatorData.selectionRectInWindowCoordinates))
+ if (!decoder.decode(textIndicatorData.selectionRectInRootViewCoordinates))
return false;
- if (!decoder.decode(textIndicatorData.textBoundingRectInWindowCoordinates))
+ if (!decoder.decode(textIndicatorData.textBoundingRectInRootViewCoordinates))
return false;
if (!decoder.decode(textIndicatorData.textRectsInBoundingRectCoordinates))
Modified: trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm (178289 => 178290)
--- trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm 2015-01-12 20:22:10 UTC (rev 178290)
@@ -3107,8 +3107,8 @@
if (!_data->_textIndicatorWindow)
_data->_textIndicatorWindow = std::make_unique<TextIndicatorWindow>(self);
- NSRect contentRect = [self.window convertRectToScreen:[self convertRect:textIndicator->textBoundingRectInWindowCoordinates() toView:nil]];
- _data->_textIndicatorWindow->setTextIndicator(textIndicator, NSRectToCGRect(contentRect), fadeOut);
+ NSRect textBoundingRectInScreenCoordinates = [self.window convertRectToScreen:[self convertRect:textIndicator->textBoundingRectInRootViewCoordinates() toView:nil]];
+ _data->_textIndicatorWindow->setTextIndicator(textIndicator, NSRectToCGRect(textBoundingRectInScreenCoordinates), fadeOut);
}
- (void)_setTextIndicatorAnimationProgress:(float)progress
Modified: trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp (178289 => 178290)
--- trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp 2015-01-12 20:02:32 UTC (rev 178289)
+++ trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp 2015-01-12 20:22:10 UTC (rev 178290)
@@ -321,7 +321,7 @@
if (!indicator)
return false;
- m_findIndicatorRect = enclosingIntRect(indicator->selectionRectInWindowCoordinates());
+ m_findIndicatorRect = enclosingIntRect(indicator->selectionRectInRootViewCoordinates());
m_webPage->send(Messages::WebPageProxy::SetTextIndicator(indicator->data(), !isShowingOverlay));
m_isShowingFindIndicator = true;
@@ -453,7 +453,7 @@
return;
if (Frame* selectedFrame = frameWithSelection(m_webPage->corePage())) {
- IntRect findIndicatorRect = selectedFrame->view()->contentsToWindow(enclosingIntRect(selectedFrame->selection().selectionBounds()));
+ IntRect findIndicatorRect = selectedFrame->view()->contentsToRootView(enclosingIntRect(selectedFrame->selection().selectionBounds()));
if (findIndicatorRect != m_findIndicatorRect)
hideFindIndicator();
