Changeset 141153 in webkit

Timestamp:

Jan 29, 2013 1:08:37 PM (11 years ago)

Author:

wangxianzhu@chromium.org

Message:

[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.

Location:

trunk/Source/WebKit/chromium

Files:

• ChangeLog (modified) (1 diff)
• src/WebViewImpl.cpp (modified) (4 diffs)
• src/WebViewImpl.h (modified) (1 diff)
• tests/WebFrameTest.cpp (modified) (3 diffs)
• tests/data/get_scale_for_zoom_into_editable_test.html (modified) (1 diff)

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2013-01-29  Xianzhu Wang  <wangxianzhu@chromium.org>
+
+        [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  <alecflett@chromium.org>
 

trunk/Source/WebKit/chromium/src/WebViewImpl.cpp

@@ -2789 +2789 @@
 
 #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();
 
@@ -2799 +2811 @@
     // 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();
 
@@ -2810 +2823 @@
     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));
-    }
-    if (textboxRectInDocumentCoordinates.height() <= m_size.height) {
+        newScroll.setX(max<int>(textboxRectInDocumentCoordinates.x(), caretInDocumentCoordinates.x() + caretInDocumentCoordinates.width() + caretPadding - viewWidth));
+    }
+    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));
-    }
-
-    bool needAnimation = false;
+        newScroll.setY(max<int>(textboxRectInDocumentCoordinates.y(), caretInDocumentCoordinates.y() + caretInDocumentCoordinates.height() + caretPadding - viewHeight));
+    }
+
+    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;
@@ -2849 +2869 @@
     if (sizeRect.contains(textboxRectInDocumentCoordinates.width(), textboxRectInDocumentCoordinates.height()) && !sizeRect.contains(textboxRect))
         needAnimation = true;
-
-    if (needAnimation)
-        startPageScaleAnimation(newOffset, false, newScale, scrollAndScaleAnimationDurationInSeconds);
-#endif
-}
+}
+#endif
 
 void WebViewImpl::advanceFocus(bool reverse)

trunk/Source/WebKit/chromium/src/WebViewImpl.h

@@ -580 +580 @@
     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);

trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp

@@ -929 +929 @@
 #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);
@@ -957 +956 @@
     // 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);
@@ -982 +992 @@
     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
 

trunk/Source/WebKit/chromium/tests/data/get_scale_for_zoom_into_editable_test.html

@@ -1 +1 @@
 <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>