Skip to content
Permalink
Browse files
[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.

Reapply r141153. Added font-size in html to ensure same caret size across platforms.

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


Canonical link: https://commits.webkit.org/126513@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@141231 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
wangxianzhu committed Jan 30, 2013
1 parent d39c197 commit 5bda28cb56ff9cc146b4e2a9886608efc7c22a3f
Showing 5 changed files with 109 additions and 59 deletions.
@@ -1,3 +1,24 @@
2013-01-30 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.

Reapply r141153. Added font-size in html to ensure same caret size across platforms.

* 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 Mark Lam <mark.lam@apple.com>

Rename AbstractDatabase to DatabaseBackend.
@@ -2788,6 +2788,18 @@ void WebViewImpl::scrollFocusedNodeIntoRect(const WebRect& rect)
}

#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 @@ void WebViewImpl::scrollFocusedNodeIntoRect(const WebRect& rect)
// 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
}
#endif

void WebViewImpl::advanceFocus(bool reverse)
{
@@ -579,6 +579,7 @@ class WebViewImpl : public WebView
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);

@@ -928,22 +928,21 @@ TEST_F(WebFrameTest, DivAutoZoomScaleFontScaleFactorTestCompositorScaling)
}
#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_F(WebFrameTest, DISABLED_DivScrollIntoEditableTest)

// 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) / 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 {
@@ -1,16 +1,14 @@
<html>
<head>
<script>
function getfocus()
{
var textfield = document.getElementById('EditBoxWithText');
textfield.focus();
textfield.setSelectionRange(textfield.value.length,textfield.value.length);
}
</script>
<style>
input {
font-size: 13px;
font-family: monospace;
}
</style>
</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>
<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>

0 comments on commit 5bda28c

Please sign in to comment.