- Revision
- 225715
- Author
- [email protected]
- Date
- 2017-12-08 16:00:14 -0800 (Fri, 08 Dec 2017)
Log Message
Wrong caret position for input field inside a fixed position parent on iOS 11
https://bugs.webkit.org/show_bug.cgi?id=176896
rdar://problem/33726145
Reviewed by Tim Horton.
Source/WebCore:
In r219668 I added code to compute a layout viewport rect in the web process, so that
after programmatic scrolling, getBoundingClientRect() would return the correct values.
However, that computation sometimes used a different visual viewport than the UI process,
resulting in a different layout viewport being set. This would happen when the keyboard
was visible, and the combination of this and zooming when focusing an input would result
in a state where the scrolling tree contained notes computed with the bad layout viewport.
This could cause apparently offset fixed elements, and bad caret positioning if those fixed
elements contained the focused input.
Fix by passing to the web process the same visual viewport rect that the UI process is using,
namely "unobscuredContentRectRespectingInputViewBounds". This was already being set in
VisibleContentRectUpdateInfo but wasn't encoded/decoded, so fix that. Set it as an optional<>
on FrameView when different from the normal visual viewport, and return it from
visualViewportRect().
Some other minor logging changes.
Test: fast/visual-viewport/ios/caret-after-focus-in-fixed.html
* page/FrameView.cpp:
(WebCore::FrameView::setVisualViewportOverrideRect):
(WebCore::FrameView::updateLayoutViewport):
(WebCore::FrameView::visualViewportRect const):
* page/FrameView.h:
* page/scrolling/mac/ScrollingTreeFixedNode.mm:
(WebCore::ScrollingTreeFixedNode::updateLayersAfterAncestorChange):
Source/WebKit:
In r219668 I added code to compute a layout viewport rect in the web process, so that
after programmatic scrolling, getBoundingClientRect() would return the correct values.
However, that computation sometimes used a different visual viewport than the UI process,
resulting in a different layout viewport being set. This would happen when the keyboard
was visible, and the combination of this and zooming when focusing an input would result
in a state where the scrolling tree contained notes computed with the bad layout viewport.
This could cause apparently offset fixed elements, and bad caret positioning if those fixed
elements contained the focused input.
Fix by passing to the web process the same visual viewport rect that the UI process is using,
namely "unobscuredContentRectRespectingInputViewBounds". This was already being set in
VisibleContentRectUpdateInfo but wasn't encoded/decoded, so fix that. Set it as an optional<>
on FrameView when different from the normal visual viewport, and return it from
visualViewportRect().
Some other minor logging changes.
* Shared/VisibleContentRectUpdateInfo.cpp:
(WebKit::VisibleContentRectUpdateInfo::encode const):
(WebKit::VisibleContentRectUpdateInfo::decode):
(WebKit::operator<<):
* Shared/VisibleContentRectUpdateInfo.h:
(WebKit::VisibleContentRectUpdateInfo::VisibleContentRectUpdateInfo):
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::updateVisibleContentRects):
LayoutTests:
Test that focuses an input inside position:fixed, then moves focus to the next
input. This was the most reliable way I could find of triggering the bug.
The test dumps the caret rect.
* fast/visual-viewport/ios/caret-after-focus-in-fixed-expected.txt: Added.
* fast/visual-viewport/ios/caret-after-focus-in-fixed.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (225714 => 225715)
--- trunk/LayoutTests/ChangeLog 2017-12-09 00:00:09 UTC (rev 225714)
+++ trunk/LayoutTests/ChangeLog 2017-12-09 00:00:14 UTC (rev 225715)
@@ -1,3 +1,18 @@
+2017-12-07 Simon Fraser <[email protected]>
+
+ Wrong caret position for input field inside a fixed position parent on iOS 11
+ https://bugs.webkit.org/show_bug.cgi?id=176896
+ rdar://problem/33726145
+
+ Reviewed by Tim Horton.
+
+ Test that focuses an input inside position:fixed, then moves focus to the next
+ input. This was the most reliable way I could find of triggering the bug.
+ The test dumps the caret rect.
+
+ * fast/visual-viewport/ios/caret-after-focus-in-fixed-expected.txt: Added.
+ * fast/visual-viewport/ios/caret-after-focus-in-fixed.html: Added.
+
2017-12-06 Simon Fraser <[email protected]>
When the iPhone keyboard is up, sometimes we never commit a stable update and re-show the caret
Added: trunk/LayoutTests/fast/visual-viewport/ios/caret-after-focus-in-fixed-expected.txt (0 => 225715)
--- trunk/LayoutTests/fast/visual-viewport/ios/caret-after-focus-in-fixed-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/visual-viewport/ios/caret-after-focus-in-fixed-expected.txt 2017-12-09 00:00:14 UTC (rev 225715)
@@ -0,0 +1,10 @@
+Focusing first field
+Moving to next field
+PASS finalCaretRect.left.toFixed(2) is "44.00"
+PASS finalCaretRect.top.toFixed(2) is "1244.00"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Checks the location of the caret after focussing inputs inside position:fixed.
+
+
Added: trunk/LayoutTests/fast/visual-viewport/ios/caret-after-focus-in-fixed.html (0 => 225715)
--- trunk/LayoutTests/fast/visual-viewport/ios/caret-after-focus-in-fixed.html (rev 0)
+++ trunk/LayoutTests/fast/visual-viewport/ios/caret-after-focus-in-fixed.html 2017-12-09 00:00:14 UTC (rev 225715)
@@ -0,0 +1,104 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+
+<html>
+<head>
+ <meta name="viewport" content="initial-scale=1.0">
+ <style>
+ body {
+ height: 2000px;
+ }
+ .fixed {
+ position: fixed;
+ top: 0;
+ width: 100%;
+ background: beige;
+ padding-top: 300px;
+ }
+ input {
+ display: block;
+ margin: 60px 30px;
+ }
+ </style>
+ <script src=""
+ <script>
+ var jsTestIsAsync = true;
+
+ function getScrollDownUIScript()
+ {
+ return `(function() {
+ uiController.immediateScrollToOffset(0, 800);
+ })();`;
+ }
+
+ function getFocusInputUIScript(x, y)
+ {
+ return `(function() {
+ uiController.didEndZoomingCallback = function() {
+ uiController.uiScriptComplete();
+ };
+ uiController.singleTapAtPoint(${x}, ${y}, function() {});
+ })();`;
+ }
+
+ function getMoveToNextInputScript()
+ {
+ return `(function() {
+ uiController.doAfterNextStablePresentationUpdate(function() {
+ uiController.uiScriptComplete(JSON.stringify(uiController.textSelectionCaretRect));
+ });
+ uiController.keyboardAccessoryBarNext();
+ })();`;
+ }
+
+ function getCaretRectScript()
+ {
+ return `(function() {
+ return JSON.stringify(uiController.textSelectionCaretRect);
+ })();`;
+ }
+
+ function toString(rect)
+ {
+ return `[${rect.left.toFixed(2)} ${rect.top.toFixed(2)} ; ${rect.width.toFixed(2)} ${rect.height.toFixed(2)}]`;
+ }
+
+ var finalCaretRect;
+
+ function runTest()
+ {
+ if (!window.testRunner || !testRunner.runUIScript)
+ return;
+
+ testRunner.runUIScript(getScrollDownUIScript(), function() {
+ window.setTimeout(function() {
+ var rect = document.getElementById('input').getBoundingClientRect();
+ debug('Focusing first field');
+ testRunner.runUIScript(getFocusInputUIScript(rect.left, rect.top + document.scrollingElement.scrollTop), function() {
+ debug('Moving to next field');
+ testRunner.runUIScript(getMoveToNextInputScript(), function() {
+ setTimeout(function() {
+ testRunner.runUIScript(getMoveToNextInputScript(), function(caretRect) {
+ finalCaretRect = JSON.parse(caretRect);
+ shouldBeEqualToString('finalCaretRect.left.toFixed(2)', '44.00');
+ shouldBeEqualToString('finalCaretRect.top.toFixed(2)', '1244.00');
+ finishJSTest();
+ });
+ }, 0);
+ });
+ });
+ }, 0);
+ });
+ }
+
+ window.addEventListener('load', runTest, false);
+ </script>
+</head>
+<body>
+<p>Checks the location of the caret after focussing inputs inside position:fixed.</p>
+<div class="fixed">
+ <input id="input" type="text"/>
+ <input type="text"/>
+</div>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (225714 => 225715)
--- trunk/Source/WebCore/ChangeLog 2017-12-09 00:00:09 UTC (rev 225714)
+++ trunk/Source/WebCore/ChangeLog 2017-12-09 00:00:14 UTC (rev 225715)
@@ -1,3 +1,38 @@
+2017-12-07 Simon Fraser <[email protected]>
+
+ Wrong caret position for input field inside a fixed position parent on iOS 11
+ https://bugs.webkit.org/show_bug.cgi?id=176896
+ rdar://problem/33726145
+
+ Reviewed by Tim Horton.
+
+ In r219668 I added code to compute a layout viewport rect in the web process, so that
+ after programmatic scrolling, getBoundingClientRect() would return the correct values.
+ However, that computation sometimes used a different visual viewport than the UI process,
+ resulting in a different layout viewport being set. This would happen when the keyboard
+ was visible, and the combination of this and zooming when focusing an input would result
+ in a state where the scrolling tree contained notes computed with the bad layout viewport.
+ This could cause apparently offset fixed elements, and bad caret positioning if those fixed
+ elements contained the focused input.
+
+ Fix by passing to the web process the same visual viewport rect that the UI process is using,
+ namely "unobscuredContentRectRespectingInputViewBounds". This was already being set in
+ VisibleContentRectUpdateInfo but wasn't encoded/decoded, so fix that. Set it as an optional<>
+ on FrameView when different from the normal visual viewport, and return it from
+ visualViewportRect().
+
+ Some other minor logging changes.
+
+ Test: fast/visual-viewport/ios/caret-after-focus-in-fixed.html
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::setVisualViewportOverrideRect):
+ (WebCore::FrameView::updateLayoutViewport):
+ (WebCore::FrameView::visualViewportRect const):
+ * page/FrameView.h:
+ * page/scrolling/mac/ScrollingTreeFixedNode.mm:
+ (WebCore::ScrollingTreeFixedNode::updateLayersAfterAncestorChange):
+
2017-12-08 Zalan Bujtas <[email protected]>
Make inline box objects IsoHeap allocated.
Modified: trunk/Source/WebCore/page/FrameView.cpp (225714 => 225715)
--- trunk/Source/WebCore/page/FrameView.cpp 2017-12-09 00:00:09 UTC (rev 225714)
+++ trunk/Source/WebCore/page/FrameView.cpp 2017-12-09 00:00:14 UTC (rev 225715)
@@ -1636,6 +1636,11 @@
setViewportConstrainedObjectsNeedLayout();
}
+void FrameView::setVisualViewportOverrideRect(std::optional<LayoutRect> rect)
+{
+ m_visualViewportOverrideRect = rect;
+}
+
LayoutSize FrameView::baseLayoutViewportSize() const
{
return renderView() ? renderView()->size() : size();
@@ -1655,11 +1660,12 @@
LOG_WITH_STREAM(Scrolling, stream << "\nFrameView " << this << " updateLayoutViewport() totalContentSize " << totalContentsSize() << " unscaledDocumentRect " << (renderView() ? renderView()->unscaledDocumentRect() : IntRect()) << " header height " << headerHeight() << " footer height " << footerHeight() << " fixed behavior " << scrollBehaviorForFixedElements());
LOG_WITH_STREAM(Scrolling, stream << "layoutViewport: " << layoutViewport);
- LOG_WITH_STREAM(Scrolling, stream << "visualViewport: " << visualViewportRect());
+ LOG_WITH_STREAM(Scrolling, stream << "visualViewport: " << visualViewportRect() << " (is override " << (bool)m_visualViewportOverrideRect << ")");
LOG_WITH_STREAM(Scrolling, stream << "stable origins: min: " << minStableLayoutViewportOrigin() << " max: "<< maxStableLayoutViewportOrigin());
if (m_layoutViewportOverrideRect) {
if (m_inProgrammaticScroll) {
+ LOG_WITH_STREAM(Scrolling, stream << "computing new override layout viewport because of programmatic scrolling");
LayoutPoint newOrigin = computeLayoutViewportOrigin(visualViewportRect(), minStableLayoutViewportOrigin(), maxStableLayoutViewportOrigin(), layoutViewport, StickToDocumentBounds);
setLayoutViewportOverrideRect(LayoutRect(newOrigin, m_layoutViewportOverrideRect.value().size()));
}
@@ -1726,6 +1732,9 @@
LayoutRect FrameView::visualViewportRect() const
{
+ if (m_visualViewportOverrideRect)
+ return m_visualViewportOverrideRect.value();
+
FloatRect visibleContentRect = this->visibleContentRect(LegacyIOSDocumentVisibleRect);
return visibleDocumentRect(visibleContentRect, headerHeight(), footerHeight(), totalContentsSize(), frameScaleFactor());
}
Modified: trunk/Source/WebCore/page/FrameView.h (225714 => 225715)
--- trunk/Source/WebCore/page/FrameView.h 2017-12-09 00:00:09 UTC (rev 225714)
+++ trunk/Source/WebCore/page/FrameView.h 2017-12-09 00:00:14 UTC (rev 225715)
@@ -250,6 +250,8 @@
// Used with delegated scrolling (i.e. iOS).
WEBCORE_EXPORT void setLayoutViewportOverrideRect(std::optional<LayoutRect>, TriggerLayoutOrNot = TriggerLayoutOrNot::Yes);
+ WEBCORE_EXPORT void setVisualViewportOverrideRect(std::optional<LayoutRect>);
+
// These are in document coordinates, unaffected by page scale (but affected by zooming).
WEBCORE_EXPORT LayoutRect layoutViewportRect() const;
WEBCORE_EXPORT LayoutRect visualViewportRect() const;
@@ -822,6 +824,7 @@
LayoutPoint m_layoutViewportOrigin;
std::optional<LayoutRect> m_layoutViewportOverrideRect;
+ std::optional<LayoutRect> m_visualViewportOverrideRect; // Used when the iOS keyboard is showing.
RefPtr<Node> m_nodeToDraw;
PaintBehavior m_paintBehavior;
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFixedNode.mm (225714 => 225715)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFixedNode.mm 2017-12-09 00:00:09 UTC (rev 225714)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFixedNode.mm 2017-12-09 00:00:14 UTC (rev 225715)
@@ -75,7 +75,7 @@
using namespace ScrollingTreeFixedNodeInternal;
FloatPoint layerPosition = m_constraints.layerPositionForViewportRect(fixedPositionRect);
- LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeFixedNode " << scrollingNodeID() << " updateLayersAfterAncestorChange: new viewport " << fixedPositionRect << " viewportRectAtLastLayout " << m_constraints.viewportRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " new offset from bottom " << (fixedPositionRect.maxY() - layerPosition.y()));
+ LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeFixedNode " << scrollingNodeID() << " updateLayersAfterAncestorChange: new viewport " << fixedPositionRect << " viewportRectAtLastLayout " << m_constraints.viewportRectAtLastLayout() << " last layer pos " << m_constraints.layerPositionAtLastLayout() << " new offset from top " << (fixedPositionRect.y() - layerPosition.y()));
layerPosition -= cumulativeDelta;
Modified: trunk/Source/WebKit/ChangeLog (225714 => 225715)
--- trunk/Source/WebKit/ChangeLog 2017-12-09 00:00:09 UTC (rev 225714)
+++ trunk/Source/WebKit/ChangeLog 2017-12-09 00:00:14 UTC (rev 225715)
@@ -1,3 +1,37 @@
+2017-12-07 Simon Fraser <[email protected]>
+
+ Wrong caret position for input field inside a fixed position parent on iOS 11
+ https://bugs.webkit.org/show_bug.cgi?id=176896
+ rdar://problem/33726145
+
+ Reviewed by Tim Horton.
+
+ In r219668 I added code to compute a layout viewport rect in the web process, so that
+ after programmatic scrolling, getBoundingClientRect() would return the correct values.
+ However, that computation sometimes used a different visual viewport than the UI process,
+ resulting in a different layout viewport being set. This would happen when the keyboard
+ was visible, and the combination of this and zooming when focusing an input would result
+ in a state where the scrolling tree contained notes computed with the bad layout viewport.
+ This could cause apparently offset fixed elements, and bad caret positioning if those fixed
+ elements contained the focused input.
+
+ Fix by passing to the web process the same visual viewport rect that the UI process is using,
+ namely "unobscuredContentRectRespectingInputViewBounds". This was already being set in
+ VisibleContentRectUpdateInfo but wasn't encoded/decoded, so fix that. Set it as an optional<>
+ on FrameView when different from the normal visual viewport, and return it from
+ visualViewportRect().
+
+ Some other minor logging changes.
+
+ * Shared/VisibleContentRectUpdateInfo.cpp:
+ (WebKit::VisibleContentRectUpdateInfo::encode const):
+ (WebKit::VisibleContentRectUpdateInfo::decode):
+ (WebKit::operator<<):
+ * Shared/VisibleContentRectUpdateInfo.h:
+ (WebKit::VisibleContentRectUpdateInfo::VisibleContentRectUpdateInfo):
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::updateVisibleContentRects):
+
2017-12-06 Simon Fraser <[email protected]>
When the iPhone keyboard is up, sometimes we never commit a stable update and re-show the caret
Modified: trunk/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp (225714 => 225715)
--- trunk/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp 2017-12-09 00:00:09 UTC (rev 225714)
+++ trunk/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp 2017-12-09 00:00:14 UTC (rev 225715)
@@ -38,6 +38,7 @@
{
encoder << m_exposedContentRect;
encoder << m_unobscuredContentRect;
+ encoder << m_unobscuredContentRectRespectingInputViewBounds;
encoder << m_unobscuredRectInScrollViewCoordinates;
encoder << m_customFixedPositionRect;
encoder << m_obscuredInsets;
@@ -61,6 +62,8 @@
return false;
if (!decoder.decode(result.m_unobscuredContentRect))
return false;
+ if (!decoder.decode(result.m_unobscuredContentRectRespectingInputViewBounds))
+ return false;
if (!decoder.decode(result.m_unobscuredRectInScrollViewCoordinates))
return false;
if (!decoder.decode(result.m_customFixedPositionRect))
@@ -112,8 +115,8 @@
ts.dumpProperty("exposedContentRect", info.exposedContentRect());
ts.dumpProperty("unobscuredContentRect", info.unobscuredContentRect());
+ ts.dumpProperty("unobscuredContentRectRespectingInputViewBounds", info.unobscuredContentRectRespectingInputViewBounds());
ts.dumpProperty("unobscuredRectInScrollViewCoordinates", info.unobscuredRectInScrollViewCoordinates());
- ts.dumpProperty("unobscuredContentRectRespectingInputViewBounds", info.unobscuredContentRectRespectingInputViewBounds());
ts.dumpProperty("customFixedPositionRect", info.customFixedPositionRect());
ts.dumpProperty("obscuredInsets", info.obscuredInsets());
ts.dumpProperty("unobscuredSafeAreaInsets", info.unobscuredSafeAreaInsets());
Modified: trunk/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h (225714 => 225715)
--- trunk/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h 2017-12-09 00:00:09 UTC (rev 225714)
+++ trunk/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h 2017-12-09 00:00:14 UTC (rev 225715)
@@ -48,8 +48,8 @@
VisibleContentRectUpdateInfo(const WebCore::FloatRect& exposedContentRect, const WebCore::FloatRect& unobscuredContentRect, const WebCore::FloatRect& unobscuredRectInScrollViewCoordinates, const WebCore::FloatRect& unobscuredContentRectRespectingInputViewBounds, const WebCore::FloatRect& customFixedPositionRect, const WebCore::FloatBoxExtent& obscuredInsets, const WebCore::FloatBoxExtent& unobscuredSafeAreaInsets, double scale, bool inStableState, bool isFirstUpdateForNewViewSize, bool isChangingObscuredInsetsInteractively, bool allowShrinkToFit, bool enclosedInScrollableAncestorView, MonotonicTime timestamp, double horizontalVelocity, double verticalVelocity, double scaleChangeRate, uint64_t lastLayerTreeTransactionId)
: m_exposedContentRect(exposedContentRect)
, m_unobscuredContentRect(unobscuredContentRect)
+ , m_unobscuredContentRectRespectingInputViewBounds(unobscuredContentRectRespectingInputViewBounds)
, m_unobscuredRectInScrollViewCoordinates(unobscuredRectInScrollViewCoordinates)
- , m_unobscuredContentRectRespectingInputViewBounds(unobscuredContentRectRespectingInputViewBounds)
, m_customFixedPositionRect(customFixedPositionRect)
, m_obscuredInsets(obscuredInsets)
, m_unobscuredSafeAreaInsets(unobscuredSafeAreaInsets)
@@ -97,8 +97,8 @@
private:
WebCore::FloatRect m_exposedContentRect;
WebCore::FloatRect m_unobscuredContentRect;
+ WebCore::FloatRect m_unobscuredContentRectRespectingInputViewBounds;
WebCore::FloatRect m_unobscuredRectInScrollViewCoordinates;
- WebCore::FloatRect m_unobscuredContentRectRespectingInputViewBounds;
WebCore::FloatRect m_customFixedPositionRect; // When visual viewports are enabled, this is the layout viewport.
WebCore::FloatBoxExtent m_obscuredInsets;
WebCore::FloatBoxExtent m_unobscuredSafeAreaInsets;
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (225714 => 225715)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2017-12-09 00:00:09 UTC (rev 225714)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2017-12-09 00:00:14 UTC (rev 225715)
@@ -2844,6 +2844,12 @@
if (m_isInStableState) {
if (frameView.frame().settings().visualViewportEnabled()) {
+ if (visibleContentRectUpdateInfo.unobscuredContentRect() != visibleContentRectUpdateInfo.unobscuredContentRectRespectingInputViewBounds())
+ frameView.setVisualViewportOverrideRect(LayoutRect(visibleContentRectUpdateInfo.unobscuredContentRectRespectingInputViewBounds()));
+ else
+ frameView.setVisualViewportOverrideRect(std::nullopt);
+
+ LOG_WITH_STREAM(VisibleRects, stream << "WebPage::updateVisibleContentRects - setLayoutViewportOverrideRect " << visibleContentRectUpdateInfo.customFixedPositionRect());
frameView.setLayoutViewportOverrideRect(LayoutRect(visibleContentRectUpdateInfo.customFixedPositionRect()));
if (selectionIsInsideFixedPositionContainer(frame)) {
// Ensure that the next layer tree commit contains up-to-date caret/selection rects.
