Diff
Modified: trunk/Source/WebKit/chromium/ChangeLog (141152 => 141153)
--- trunk/Source/WebKit/chromium/ChangeLog 2013-01-29 21:04:46 UTC (rev 141152)
+++ trunk/Source/WebKit/chromium/ChangeLog 2013-01-29 21:08:37 UTC (rev 141153)
@@ -1,3 +1,22 @@
+2013-01-29 Xianzhu Wang <[email protected]>
+
+ [Chromium] Correct zoom for focused node when using compositor scaling
+ https://bugs.webkit.org/show_bug.cgi?id=107599
+
+ Reviewed by Adam Barth.
+
+ When applyDeviceScaleFactorInCompositor, targetScale should exclude device scale factor.
+ When applyPageScaleFactorInCompositor, caret size and content sizes are in css pixels and they should be in the viewport of the new scale.
+
+ * src/WebViewImpl.cpp:
+ (WebKit::WebViewImpl::scrollFocusedNodeIntoRect):
+ (WebKit):
+ (WebKit::WebViewImpl::computeScaleAndScrollForFocusedNode): Extracted from scrollFocusedNodeIntoRect() to ease testing.
+ * src/WebViewImpl.h:
+ (WebViewImpl):
+ * tests/WebFrameTest.cpp: Updated test DivScrollEditableTest
+ * tests/data/get_scale_for_zoom_into_editable_test.html: Moved the logic of onload script (which seems not to work) into WebFrameTest.cpp.
+
2013-01-29 Alec Flett <[email protected]>
IndexedDB: Pass metadata in to IDBOpenDBRequest.onUpgradeNeeded/onSuccess
Modified: trunk/Source/WebKit/chromium/src/WebViewImpl.cpp (141152 => 141153)
--- trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2013-01-29 21:04:46 UTC (rev 141152)
+++ trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2013-01-29 21:08:37 UTC (rev 141153)
@@ -2788,6 +2788,18 @@
}
#if ENABLE(GESTURE_EVENTS)
+ float scale;
+ IntPoint scroll;
+ bool needAnimation;
+ computeScaleAndScrollForFocusedNode(focusedNode, scale, scroll, needAnimation);
+ if (needAnimation)
+ startPageScaleAnimation(scroll, false, scale, scrollAndScaleAnimationDurationInSeconds);
+#endif
+}
+
+#if ENABLE(GESTURE_EVENTS)
+void WebViewImpl::computeScaleAndScrollForFocusedNode(Node* focusedNode, float& newScale, IntPoint& newScroll, bool& needAnimation)
+{
focusedNode->document()->updateLayoutIgnorePendingStylesheets();
// 'caret' is rect encompassing the blinking cursor.
@@ -2798,61 +2810,66 @@
// Pick a scale which is reasonably readable. This is the scale at which
// the caret height will become minReadableCaretHeight (adjusted for dpi
// and font scale factor).
- float targetScale = deviceScaleFactor();
+ float targetScale = settingsImpl()->applyDeviceScaleFactorInCompositor() ? 1 : deviceScaleFactor();
#if ENABLE(TEXT_AUTOSIZING)
if (page() && page()->settings())
targetScale *= page()->settings()->textAutosizingFontScaleFactor();
#endif
- const float newScale = clampPageScaleFactorToLimits(pageScaleFactor() * minReadableCaretHeight * targetScale / caret.height);
+ const float caretHeight = settingsImpl()->applyPageScaleFactorInCompositor() ? caret.height : caret.height / pageScaleFactor();
+ newScale = clampPageScaleFactorToLimits(minReadableCaretHeight * targetScale / caretHeight);
const float deltaScale = newScale / pageScaleFactor();
// Convert the rects to absolute space in the new scale.
IntRect textboxRectInDocumentCoordinates = textboxRect;
textboxRectInDocumentCoordinates.move(mainFrame()->scrollOffset());
- textboxRectInDocumentCoordinates.scale(deltaScale);
IntRect caretInDocumentCoordinates = caret;
caretInDocumentCoordinates.move(mainFrame()->scrollOffset());
- caretInDocumentCoordinates.scale(deltaScale);
- IntPoint newOffset;
- if (textboxRectInDocumentCoordinates.width() <= m_size.width) {
+ int viewWidth = m_size.width;
+ int viewHeight = m_size.height;
+ if (settingsImpl()->applyPageScaleFactorInCompositor()) {
+ viewWidth /= newScale;
+ viewHeight /= newScale;
+ } else {
+ textboxRectInDocumentCoordinates.scale(deltaScale);
+ caretInDocumentCoordinates.scale(deltaScale);
+ }
+
+ if (textboxRectInDocumentCoordinates.width() <= viewWidth) {
// Field is narrower than screen. Try to leave padding on left so field's
// label is visible, but it's more important to ensure entire field is
// onscreen.
- int idealLeftPadding = m_size.width * leftBoxRatio;
- int maxLeftPaddingKeepingBoxOnscreen = m_size.width - textboxRectInDocumentCoordinates.width();
- newOffset.setX(textboxRectInDocumentCoordinates.x() - min<int>(idealLeftPadding, maxLeftPaddingKeepingBoxOnscreen));
+ int idealLeftPadding = viewWidth * leftBoxRatio;
+ int maxLeftPaddingKeepingBoxOnscreen = viewWidth - textboxRectInDocumentCoordinates.width();
+ newScroll.setX(textboxRectInDocumentCoordinates.x() - min<int>(idealLeftPadding, maxLeftPaddingKeepingBoxOnscreen));
} else {
// Field is wider than screen. Try to left-align field, unless caret would
// be offscreen, in which case right-align the caret.
- newOffset.setX(max<int>(textboxRectInDocumentCoordinates.x(), caretInDocumentCoordinates.x() + caretInDocumentCoordinates.width() + caretPadding - m_size.width));
+ newScroll.setX(max<int>(textboxRectInDocumentCoordinates.x(), caretInDocumentCoordinates.x() + caretInDocumentCoordinates.width() + caretPadding - viewWidth));
}
- if (textboxRectInDocumentCoordinates.height() <= m_size.height) {
+ if (textboxRectInDocumentCoordinates.height() <= viewHeight) {
// Field is shorter than screen. Vertically center it.
- newOffset.setY(textboxRectInDocumentCoordinates.y() - (m_size.height - textboxRectInDocumentCoordinates.height()) / 2);
+ newScroll.setY(textboxRectInDocumentCoordinates.y() - (viewHeight - textboxRectInDocumentCoordinates.height()) / 2);
} else {
// Field is taller than screen. Try to top align field, unless caret would
// be offscreen, in which case bottom-align the caret.
- newOffset.setY(max<int>(textboxRectInDocumentCoordinates.y(), caretInDocumentCoordinates.y() + caretInDocumentCoordinates.height() + caretPadding - m_size.height));
+ newScroll.setY(max<int>(textboxRectInDocumentCoordinates.y(), caretInDocumentCoordinates.y() + caretInDocumentCoordinates.height() + caretPadding - viewHeight));
}
- bool needAnimation = false;
+ needAnimation = false;
// If we are at less than the target zoom level, zoom in.
if (deltaScale > minScaleChangeToTriggerZoom)
needAnimation = true;
// If the caret is offscreen, then animate.
- IntRect sizeRect(0, 0, m_size.width, m_size.height);
+ IntRect sizeRect(0, 0, viewWidth, viewHeight);
if (!sizeRect.contains(caret))
needAnimation = true;
// If the box is partially offscreen and it's possible to bring it fully
// onscreen, then animate.
if (sizeRect.contains(textboxRectInDocumentCoordinates.width(), textboxRectInDocumentCoordinates.height()) && !sizeRect.contains(textboxRect))
needAnimation = true;
-
- if (needAnimation)
- startPageScaleAnimation(newOffset, false, newScale, scrollAndScaleAnimationDurationInSeconds);
+}
#endif
-}
void WebViewImpl::advanceFocus(bool reverse)
{
Modified: trunk/Source/WebKit/chromium/src/WebViewImpl.h (141152 => 141153)
--- trunk/Source/WebKit/chromium/src/WebViewImpl.h 2013-01-29 21:04:46 UTC (rev 141152)
+++ trunk/Source/WebKit/chromium/src/WebViewImpl.h 2013-01-29 21:08:37 UTC (rev 141153)
@@ -579,6 +579,7 @@
void computeScaleAndScrollForHitRect(const WebRect& hitRect, AutoZoomType, float& scale, WebPoint& scroll, bool& isAnchor);
WebCore::Node* bestTouchLinkNode(const WebGestureEvent& touchEvent);
void enableTouchHighlight(const WebGestureEvent& touchEvent);
+ void computeScaleAndScrollForFocusedNode(WebCore::Node* focusedNode, float& scale, WebCore::IntPoint& scroll, bool& needAnimation);
#endif
void animateZoomAroundPoint(const WebCore::IntPoint&, AutoZoomType);
Modified: trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp (141152 => 141153)
--- trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2013-01-29 21:04:46 UTC (rev 141152)
+++ trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2013-01-29 21:08:37 UTC (rev 141153)
@@ -928,22 +928,21 @@
}
#endif
-// This test depends on code that is compiled conditionally. We likely need to
-// add the proper ifdef when re-enabling it. See
-// https://bugs.webkit.org/show_bug.cgi?id=98558
-TEST_F(WebFrameTest, DISABLED_DivScrollIntoEditableTest)
+TEST_F(WebFrameTest, DivScrollIntoEditableTest)
{
registerMockedHttpURLLoad("get_scale_for_zoom_into_editable_test.html");
- int viewportWidth = 640;
- int viewportHeight = 480;
+ int viewportWidth = 450;
+ int viewportHeight = 300;
float leftBoxRatio = 0.3f;
int caretPadding = 10;
- int minReadableCaretHeight = 18;
+ float minReadableCaretHeight = 18.0f;
WebKit::WebView* webView = FrameTestHelpers::createWebViewAndLoad(m_baseURL + "get_scale_for_zoom_into_editable_test.html");
+ webView->settings()->setApplyDeviceScaleFactorInCompositor(true);
+ webView->settings()->setApplyPageScaleFactorInCompositor(true);
webView->enableFixedLayoutMode(true);
webView->resize(WebSize(viewportWidth, viewportHeight));
- webView->setPageScaleFactorLimits(1, 10);
+ webView->setPageScaleFactorLimits(1, 4);
webView->layout();
webView->setDeviceScaleFactor(1.5f);
webView->settings()->setAutoZoomFocusedNodeToLegibleScale(true);
@@ -956,46 +955,60 @@
// Test scrolling the focused node
// The edit box is shorter and narrower than the viewport when legible.
+ webView->advanceFocus(false);
+ // Set the caret to the end of the input box.
+ webView->mainFrame()->document().getElementById("EditBoxWithText").to<WebInputElement>().setSelectionRange(1000, 1000);
setScaleAndScrollAndLayout(webView, WebPoint(0, 0), 1);
WebRect rect, caret;
webViewImpl->selectionBounds(caret, rect);
- webView->scrollFocusedNodeIntoRect(rect);
+
+ float scale;
+ WebCore::IntPoint scroll;
+ bool needAnimation;
+ webViewImpl->computeScaleAndScrollForFocusedNode(webViewImpl->focusedWebCoreNode(), scale, scroll, needAnimation);
+ EXPECT_TRUE(needAnimation);
// The edit box should be left aligned with a margin for possible label.
- int hScroll = editBoxWithText.x * webView->pageScaleFactor() - leftBoxRatio * viewportWidth;
- EXPECT_EQ(hScroll, webView->mainFrame()->scrollOffset().width);
- int vScroll = editBoxWithText.y * webView->pageScaleFactor() - (viewportHeight - editBoxWithText.height * webView->pageScaleFactor()) / 2;
- EXPECT_EQ(vScroll, webView->mainFrame()->scrollOffset().height);
- EXPECT_FLOAT_EQ(webView->deviceScaleFactor() * minReadableCaretHeight / caret.height, webView->pageScaleFactor());
+ int hScroll = editBoxWithText.x - leftBoxRatio * viewportWidth / scale;
+ EXPECT_NEAR(hScroll, scroll.x(), 1);
+ int vScroll = editBoxWithText.y - (viewportHeight / scale - editBoxWithText.height * scale) / 2;
+ EXPECT_NEAR(vScroll, scroll.y(), 1);
+ EXPECT_NEAR(minReadableCaretHeight / caret.height, scale, 0.1);
// The edit box is wider than the viewport when legible.
- webView->setDeviceScaleFactor(4);
+ viewportWidth = 200;
+ viewportHeight = 150;
+ webView->resize(WebSize(viewportWidth, viewportHeight));
setScaleAndScrollAndLayout(webView, WebPoint(0, 0), 1);
webViewImpl->selectionBounds(caret, rect);
- webView->scrollFocusedNodeIntoRect(rect);
+ webViewImpl->computeScaleAndScrollForFocusedNode(webViewImpl->focusedWebCoreNode(), scale, scroll, needAnimation);
+ EXPECT_TRUE(needAnimation);
// The caret should be right aligned since the caret would be offscreen when the edit box is left aligned.
- hScroll = (caret.x + caret.width) * webView->pageScaleFactor() + caretPadding - viewportWidth;
- EXPECT_EQ(hScroll, webView->mainFrame()->scrollOffset().width);
- EXPECT_FLOAT_EQ(webView->deviceScaleFactor() * minReadableCaretHeight / caret.height, webView->pageScaleFactor());
+ hScroll = caret.x + caret.width + caretPadding - viewportWidth / scale;
+ EXPECT_NEAR(hScroll, scroll.x(), 1);
+ EXPECT_NEAR(minReadableCaretHeight / caret.height, scale, 0.1);
setScaleAndScrollAndLayout(webView, WebPoint(0, 0), 1);
// Move focus to edit box with text.
webView->advanceFocus(false);
webViewImpl->selectionBounds(caret, rect);
- webView->scrollFocusedNodeIntoRect(rect);
+ webViewImpl->computeScaleAndScrollForFocusedNode(webViewImpl->focusedWebCoreNode(), scale, scroll, needAnimation);
+ EXPECT_TRUE(needAnimation);
// The edit box should be left aligned.
- hScroll = editBoxWithNoText.x * webView->pageScaleFactor();
- EXPECT_EQ(hScroll, webView->mainFrame()->scrollOffset().width);
- vScroll = editBoxWithNoText.y * webView->pageScaleFactor() - (viewportHeight - editBoxWithNoText.height * webView->pageScaleFactor()) / 2;
- EXPECT_EQ(vScroll, webView->mainFrame()->scrollOffset().height);
- EXPECT_FLOAT_EQ(webView->deviceScaleFactor() * minReadableCaretHeight / caret.height, webView->pageScaleFactor());
+ hScroll = editBoxWithNoText.x;
+ EXPECT_NEAR(hScroll, scroll.x(), 1);
+ vScroll = editBoxWithNoText.y - (viewportHeight / scale - editBoxWithNoText.height) / 2;
+ EXPECT_NEAR(vScroll, scroll.y(), 1);
+ EXPECT_NEAR(minReadableCaretHeight / caret.height, scale, 0.1);
+ setScaleAndScrollAndLayout(webViewImpl, scroll, scale);
+
// Move focus back to the first edit box.
webView->advanceFocus(true);
- webViewImpl->selectionBounds(caret, rect);
+ webViewImpl->computeScaleAndScrollForFocusedNode(webViewImpl->focusedWebCoreNode(), scale, scroll, needAnimation);
// The position should have stayed the same since this box was already on screen with the right scale.
- EXPECT_EQ(vScroll, webView->mainFrame()->scrollOffset().height);
- EXPECT_EQ(hScroll, webView->mainFrame()->scrollOffset().width);
+ EXPECT_FALSE(needAnimation);
}
+
#endif
class TestReloadDoesntRedirectWebFrameClient : public WebFrameClient {
Modified: trunk/Source/WebKit/chromium/tests/data/get_scale_for_zoom_into_editable_test.html (141152 => 141153)
--- trunk/Source/WebKit/chromium/tests/data/get_scale_for_zoom_into_editable_test.html 2013-01-29 21:04:46 UTC (rev 141152)
+++ trunk/Source/WebKit/chromium/tests/data/get_scale_for_zoom_into_editable_test.html 2013-01-29 21:08:37 UTC (rev 141153)
@@ -1,16 +1,8 @@
<html>
<head>
-<script>
-function getfocus()
-{
-var textfield = document.getElementById('EditBoxWithText');
-textfield.focus();
-textfield.setSelectionRange(textfield.value.length,textfield.value.length);
-}
-</script>
</head>
- <body _onload_="getfocus()">
- <input id="EditBoxWithText" style=" position: absolute; left: 200px; top: 200px; width: 250; height:20" value = "Long text to fill the edit box" type="text">
- <input id="EditBoxWithNoText" style=" position: absolute; left: 200px; top: 250px; width: 250; height:20" type="text">
+ <body>
+ <input id="EditBoxWithText" style=" position: absolute; left: 200px; top: 200px; width: 250px; height:20" value = "Long long long long text to fill the edit box" type="text">
+ <input id="EditBoxWithNoText" style=" position: absolute; left: 200px; top: 250px; width: 250px; height:20" type="text">
</body>
</html>
