- Revision
- 168763
- Author
- [email protected]
- Date
- 2014-05-13 17:13:56 -0700 (Tue, 13 May 2014)
Log Message
REGRESSION (topContentInset): Searching through Facebook Messenger's chat causes
scrolling in News Feed
https://bugs.webkit.org/show_bug.cgi?id=132889
-and corresponding-
<rdar://problem/16715716>
Reviewed by Simon Fraser.
Source/WebCore:
First of all, scrollOffsetRelativeToDocument() was very poorly named. This patch
re-names it to the much more accurate documentScrollOffsetRelativeToViewOrigin().
Re-naming it makes it clear that ONE call site was not getting the right offset.
That call site does not want to know the document’s position relative to the view
origin, but rather it wants to know the Document’s position relative to the
scrolling origin.
Export new name.
* WebCore.exp.in:
Use newly re-named documentScrollPositionRelativeToViewOrigin().
* page/FrameView.cpp:
(WebCore::FrameView::convertToRenderer):
* platform/ScrollView.cpp:
(WebCore::ScrollView::documentScrollOffsetRelativeToViewOrigin):
(WebCore::ScrollView::documentScrollPositionRelativeToViewOrigin):
(WebCore::ScrollView::documentScrollOffsetRelativeToScrollableAreaOrigin):
(WebCore::ScrollView::rootViewToContents):
(WebCore::ScrollView::windowToContents):
(WebCore::ScrollView::scrollOffsetRelativeToDocument): Deleted.
(WebCore::ScrollView::scrollPositionRelativeToDocument): Deleted.
* platform/ScrollView.h:
THIS is the spot that needs the new function,
documentScrollOffsetRelativeToScrollableAreaOrigin()()
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollRectToVisible):
Source/WebKit2:
Re-name scrollOffsetRelativeToDocument() to
documentScrollPositionRelativeToViewOrigin().
* WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::rectsForTextMatches):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::determinePrimarySnapshottedPlugIn):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (168762 => 168763)
--- trunk/Source/WebCore/ChangeLog 2014-05-14 00:10:44 UTC (rev 168762)
+++ trunk/Source/WebCore/ChangeLog 2014-05-14 00:13:56 UTC (rev 168763)
@@ -1,3 +1,41 @@
+2014-05-13 Beth Dakin <[email protected]>
+
+ REGRESSION (topContentInset): Searching through Facebook Messenger's chat causes
+ scrolling in News Feed
+ https://bugs.webkit.org/show_bug.cgi?id=132889
+ -and corresponding-
+ <rdar://problem/16715716>
+
+ Reviewed by Simon Fraser.
+
+ First of all, scrollOffsetRelativeToDocument() was very poorly named. This patch
+ re-names it to the much more accurate documentScrollOffsetRelativeToViewOrigin().
+ Re-naming it makes it clear that ONE call site was not getting the right offset.
+ That call site does not want to know the document’s position relative to the view
+ origin, but rather it wants to know the Document’s position relative to the
+ scrolling origin.
+
+ Export new name.
+ * WebCore.exp.in:
+
+ Use newly re-named documentScrollPositionRelativeToViewOrigin().
+ * page/FrameView.cpp:
+ (WebCore::FrameView::convertToRenderer):
+ * platform/ScrollView.cpp:
+ (WebCore::ScrollView::documentScrollOffsetRelativeToViewOrigin):
+ (WebCore::ScrollView::documentScrollPositionRelativeToViewOrigin):
+ (WebCore::ScrollView::documentScrollOffsetRelativeToScrollableAreaOrigin):
+ (WebCore::ScrollView::rootViewToContents):
+ (WebCore::ScrollView::windowToContents):
+ (WebCore::ScrollView::scrollOffsetRelativeToDocument): Deleted.
+ (WebCore::ScrollView::scrollPositionRelativeToDocument): Deleted.
+ * platform/ScrollView.h:
+
+ THIS is the spot that needs the new function,
+ documentScrollOffsetRelativeToScrollableAreaOrigin()()
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::scrollRectToVisible):
+
2014-05-13 Dean Jackson <[email protected]>
[iOS] Page scale update messages for media controls should only fire at the end of zooming
Modified: trunk/Source/WebCore/WebCore.exp.in (168762 => 168763)
--- trunk/Source/WebCore/WebCore.exp.in 2014-05-14 00:10:44 UTC (rev 168762)
+++ trunk/Source/WebCore/WebCore.exp.in 2014-05-14 00:13:56 UTC (rev 168763)
@@ -1507,7 +1507,7 @@
__ZNK7WebCore10ScrollView18contentsToRootViewERKNS_8IntPointE
__ZNK7WebCore10ScrollView18rootViewToContentsERKNS_8IntPointE
__ZNK7WebCore10ScrollView23rootViewToTotalContentsERKNS_8IntPointE
-__ZNK7WebCore10ScrollView30scrollOffsetRelativeToDocumentEv
+__ZNK7WebCore10ScrollView40documentScrollOffsetRelativeToViewOriginEv
__ZNK7WebCore10StorageMap6lengthEv
__ZNK7WebCore10StorageMap7getItemERKN3WTF6StringE
__ZNK7WebCore10StorageMap8containsERKN3WTF6StringE
Modified: trunk/Source/WebCore/page/FrameView.cpp (168762 => 168763)
--- trunk/Source/WebCore/page/FrameView.cpp 2014-05-14 00:10:44 UTC (rev 168762)
+++ trunk/Source/WebCore/page/FrameView.cpp 2014-05-14 00:13:56 UTC (rev 168763)
@@ -3879,7 +3879,7 @@
// Convert from FrameView coords into page ("absolute") coordinates.
if (!delegatesScrolling())
- rect.moveBy(scrollPositionRelativeToDocument());
+ rect.moveBy(documentScrollPositionRelativeToViewOrigin());
// FIXME: we don't have a way to map an absolute rect down to a local quad, so just
// move the rect for now.
@@ -3903,7 +3903,7 @@
// Convert from FrameView coords into page ("absolute") coordinates.
if (!delegatesScrolling())
- point = point + scrollPositionRelativeToDocument();
+ point = point + documentScrollPositionRelativeToViewOrigin();
return roundedIntPoint(renderer->absoluteToLocal(point, UseTransforms));
}
Modified: trunk/Source/WebCore/platform/ScrollView.cpp (168762 => 168763)
--- trunk/Source/WebCore/platform/ScrollView.cpp 2014-05-14 00:10:44 UTC (rev 168762)
+++ trunk/Source/WebCore/platform/ScrollView.cpp 2014-05-14 00:13:56 UTC (rev 168763)
@@ -412,18 +412,22 @@
return newScrollPosition;
}
-IntSize ScrollView::scrollOffsetRelativeToDocument() const
+IntSize ScrollView::documentScrollOffsetRelativeToViewOrigin() const
{
- IntSize scrollOffset = this->scrollOffset();
- return IntSize(scrollOffset.width(), scrollOffset.height() - headerHeight() - topContentInset());
+ return scrollOffset() - IntSize(0, headerHeight() + topContentInset());
}
-IntPoint ScrollView::scrollPositionRelativeToDocument() const
+IntPoint ScrollView::documentScrollPositionRelativeToViewOrigin() const
{
IntPoint scrollPosition = this->scrollPosition();
return IntPoint(scrollPosition.x(), scrollPosition.y() - headerHeight() - topContentInset());
}
+IntSize ScrollView::documentScrollOffsetRelativeToScrollableAreaOrigin() const
+{
+ return scrollOffset() - IntSize(0, headerHeight());
+}
+
int ScrollView::scrollSize(ScrollbarOrientation orientation) const
{
// If no scrollbars are present, it does not indicate content is not be scrollable.
@@ -822,7 +826,7 @@
return convertFromRootView(rootViewPoint);
IntPoint viewPoint = convertFromRootView(rootViewPoint);
- return viewPoint + scrollOffsetRelativeToDocument();
+ return viewPoint + documentScrollOffsetRelativeToViewOrigin();
}
IntPoint ScrollView::contentsToRootView(const IntPoint& contentsPoint) const
@@ -840,7 +844,7 @@
return convertFromRootView(rootViewRect);
IntRect viewRect = convertFromRootView(rootViewRect);
- viewRect.move(scrollOffsetRelativeToDocument());
+ viewRect.move(documentScrollOffsetRelativeToViewOrigin());
return viewRect;
}
@@ -869,7 +873,7 @@
return convertFromContainingWindow(windowPoint);
IntPoint viewPoint = convertFromContainingWindow(windowPoint);
- return viewPoint + scrollOffsetRelativeToDocument();
+ return viewPoint + documentScrollOffsetRelativeToViewOrigin();
}
IntPoint ScrollView::contentsToWindow(const IntPoint& contentsPoint) const
@@ -887,7 +891,7 @@
return convertFromContainingWindow(windowRect);
IntRect viewRect = convertFromContainingWindow(windowRect);
- viewRect.move(scrollOffsetRelativeToDocument());
+ viewRect.move(documentScrollOffsetRelativeToViewOrigin());
return viewRect;
}
Modified: trunk/Source/WebCore/platform/ScrollView.h (168762 => 168763)
--- trunk/Source/WebCore/platform/ScrollView.h 2014-05-14 00:10:44 UTC (rev 168762)
+++ trunk/Source/WebCore/platform/ScrollView.h 2014-05-14 00:13:56 UTC (rev 168763)
@@ -241,13 +241,22 @@
IntPoint actualScrollPosition() const { return unobscuredContentRect().location(); }
#endif
- // scrollOffset() anchors its (0,0) point at the top end of the header if this ScrollableArea
- // has a header, so it is relative to the totalContentsSize(). scrollOffsetRelativeToDocument()
- // anchors (0,0) at the top of the Document, which will be beneath any headers, so it is relative
- // to contentsSize().
- IntSize scrollOffsetRelativeToDocument() const;
- IntPoint scrollPositionRelativeToDocument() const;
+ // scrollOffset() anchors its (0,0) point at the ScrollableArea's origin. When the Page has a
+ // header, the header is positioned at (0,0), ABOVE the start of the Document. So when a page with
+ // a header is pinned to the top, the scrollOffset() is (0,0), but the Document is actually at
+ // (0, -headerHeight()). documentScrollOffsetRelativeToScrollableAreaOrigin() will return this
+ // version of the offset, which tracks the top of Document relative to where scrolling was achored.
+ IntSize documentScrollOffsetRelativeToScrollableAreaOrigin() const;
+ // scrollOffset() anchors its (0,0) point at the ScrollableArea's origin. The top of the scrolling
+ // layer does not represent the top of the view when there is a topContentInset. Additionally, as
+ // detailed above, the origin of the scrolling layer also does not necessarily correspond with the
+ // top of the document anyway, since there could also be header. documentScrollOffsetRelativeToViewOrigin()
+ // will return a version of the current scroll offset which tracks the top of the Document
+ // relative to the very top of the view.
+ IntSize documentScrollOffsetRelativeToViewOrigin() const;
+ IntPoint documentScrollPositionRelativeToViewOrigin() const;
+
virtual IntSize overhangAmount() const override;
void cacheCurrentScrollPosition() { m_cachedScrollPosition = scrollPosition(); }
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (168762 => 168763)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2014-05-14 00:10:44 UTC (rev 168762)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2014-05-14 00:13:56 UTC (rev 168763)
@@ -2409,8 +2409,8 @@
#if !PLATFORM(IOS)
LayoutRect viewRect = frameView.visibleContentRect();
LayoutRect visibleRectRelativeToDocument = viewRect;
- IntSize scrollOffsetRelativeToDocument = frameView.scrollOffsetRelativeToDocument();
- visibleRectRelativeToDocument.setLocation(IntPoint(scrollOffsetRelativeToDocument.width(), scrollOffsetRelativeToDocument.height()));
+ IntSize documentScrollOffsetRelativeToScrollableAreaOrigin = frameView.documentScrollOffsetRelativeToScrollableAreaOrigin();
+ visibleRectRelativeToDocument.setLocation(IntPoint(documentScrollOffsetRelativeToScrollableAreaOrigin.width(), documentScrollOffsetRelativeToScrollableAreaOrigin.height()));
#else
LayoutRect viewRect = frameView.unobscuredContentRect();
LayoutRect visibleRectRelativeToDocument = viewRect;
Modified: trunk/Source/WebKit2/ChangeLog (168762 => 168763)
--- trunk/Source/WebKit2/ChangeLog 2014-05-14 00:10:44 UTC (rev 168762)
+++ trunk/Source/WebKit2/ChangeLog 2014-05-14 00:13:56 UTC (rev 168763)
@@ -1,3 +1,20 @@
+2014-05-13 Beth Dakin <[email protected]>
+
+ REGRESSION (topContentInset): Searching through Facebook Messenger's chat causes
+ scrolling in News Feed
+ https://bugs.webkit.org/show_bug.cgi?id=132889
+ -and corresponding-
+ <rdar://problem/16715716>
+
+ Reviewed by Simon Fraser.
+
+ Re-name scrollOffsetRelativeToDocument() to
+ documentScrollPositionRelativeToViewOrigin().
+ * WebProcess/WebPage/FindController.cpp:
+ (WebKit::FindController::rectsForTextMatches):
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::determinePrimarySnapshottedPlugIn):
+
2014-05-13 Dean Jackson <[email protected]>
[iOS] Page scale update messages for media controls should only fire at the end of zooming
Modified: trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp (168762 => 168763)
--- trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp 2014-05-14 00:10:44 UTC (rev 168762)
+++ trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp 2014-05-14 00:13:56 UTC (rev 168763)
@@ -418,7 +418,7 @@
IntRect visibleRect = frame->view()->visibleContentRect();
Vector<IntRect> frameRects = document->markers().renderedRectsForMarkers(DocumentMarker::TextMatch);
- IntPoint frameOffset(-frame->view()->scrollOffsetRelativeToDocument().width(), -frame->view()->scrollOffsetRelativeToDocument().height());
+ IntPoint frameOffset(-frame->view()->documentScrollOffsetRelativeToViewOrigin().width(), -frame->view()->documentScrollOffsetRelativeToViewOrigin().height());
frameOffset = frame->view()->convertToContainingWindow(frameOffset);
for (Vector<IntRect>::iterator it = frameRects.begin(), end = frameRects.end(); it != end; ++it) {
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp (168762 => 168763)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2014-05-14 00:10:44 UTC (rev 168762)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2014-05-14 00:13:56 UTC (rev 168763)
@@ -4395,7 +4395,7 @@
IntRect plugInRectRelativeToView = plugInImageElement.clientRect();
if (plugInRectRelativeToView.isEmpty())
continue;
- IntSize scrollOffset = mainFrame.view()->scrollOffsetRelativeToDocument();
+ IntSize scrollOffset = mainFrame.view()->documentScrollOffsetRelativeToViewOrigin();
IntRect plugInRectRelativeToTopDocument(plugInRectRelativeToView.location() + scrollOffset, plugInRectRelativeToView.size());
if (!plugInRectRelativeToTopDocument.intersects(searchRect))
continue;
