Title: [171891] trunk
Revision
171891
Author
[email protected]
Date
2014-07-31 15:32:40 -0700 (Thu, 31 Jul 2014)

Log Message

Hit-testing broken in WebKit 1 views with AppKit's contentInsets
https://bugs.webkit.org/show_bug.cgi?id=135434
-and corresponding-
<rdar://problem/17850323>

Reviewed by Benjamin Poulain.


Source/WebCore: 
AppKit's contentInsets are factored into scroll positions and mouse positions, but
in WebCore, we generally want all of those things to be factored out so that, for
example, the scroll position of a document pinned to the top is the same whether
or not there is a contentInset. So to fix hit-testing in WebKit 1 views with
contentInsets, this patch factors the inset out or into of all coordinate
conversion methods just like we do for the WebKit2 contentInset.
        
This patch also adds the ability to test WK1 platformContentInsets with
window.internals, and it re-names convertFromRenderer to 
convertFromRendererToContainingView and also re-names convertToRenderer to
convertFromContainingViewToRenderer.

ScrollView::topContentInset() takes an optional parameter indicating whether the
caller wants the WebCore::Page contentInset or the platform content inset. It’s
necessary to distinguish between these cases because there is a lot of code that
only wants the WebCore::Page contentInset since that feature is actually
implemented in WebCore as opposed to being implemented at the platform level.
* WebCore.exp.in:
* page/FrameView.cpp:
(WebCore::FrameView::topContentInset):

For layout test purposes, set the platforTopContentInset here if there is a 
platformWidget().
(WebCore::FrameView::topContentInsetDidChange):

Re-named functions, also all coordinate conversion functions call 
topContentInset(ManualOrPlatformContentInset)
(WebCore::FrameView::convertFromContainingViewToRenderer):
(WebCore::FrameView::convertToContainingView):
(WebCore::FrameView::convertFromContainingView):
(WebCore::FrameView::convertFromRenderer): Deleted.
(WebCore::FrameView::convertToRenderer): Deleted.
* page/FrameView.h:
* page/Page.cpp:
(WebCore::Page::setTopContentInset):
* platform/ScrollView.cpp:
(WebCore::ScrollView::documentScrollOffsetRelativeToViewOrigin):
(WebCore::ScrollView::documentScrollPositionRelativeToViewOrigin):
(WebCore::ScrollView::contentsToRootView):
(WebCore::ScrollView::rootViewToTotalContents):
(WebCore::ScrollView::contentsToWindow):
(WebCore::ScrollView::platformTopContentInset):
(WebCore::ScrollView::platformSetTopContentInset):
* platform/ScrollView.h:
(WebCore::ScrollView::topContentInset):
* platform/ios/ScrollViewIOS.mm:
(WebCore::ScrollView::platformTopContentInset):
(WebCore::ScrollView::platformSetTopContentInset):

Implement new platform inset-related functions using AppKit's implementation.
* platform/mac/ScrollViewMac.mm:
(WebCore::ScrollView::platformTopContentInset):
(WebCore::ScrollView::platformSetTopContentInset):
        
When we set the scroll position for the documentView, we have to factor the inset
back into the WebCore scroll position.
(WebCore::ScrollView::platformSetScrollPosition):
        
Re-named functions.
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::convertFromScrollbarToContainingView):
(WebCore::RenderLayer::convertFromContainingViewToScrollbar):
* rendering/RenderListBox.cpp:
(WebCore::RenderListBox::convertFromScrollbarToContainingView):
(WebCore::RenderListBox::convertFromContainingViewToScrollbar):

Source/WebKit2: 
Re-named functions.
* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::convertFromScrollbarToContainingView):
(WebKit::PDFPlugin::convertFromContainingViewToScrollbar):

LayoutTests: 
These hit-testing related contentInset tests will now work for mac on wk1 on 
Yosemite and wk2.

* platform/mac-wk1/TestExpectations:
* platform/mac-wk2/tiled-drawing/content-inset-hit-testing-expected.txt: Removed.
* platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame-expected.txt: Removed.
* platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame.html: Removed.
* platform/mac-wk2/tiled-drawing/content-inset-hit-testing.html: Removed.
* platform/mac-wk2/tiled-drawing/resources/iframe-to-hit-test.html: Removed.
* platform/mac/fast/events/content-inset-hit-testing-expected.txt: Copied from platform/mac-wk2/tiled-drawing/content-inset-hit-testing-expected.txt.
* platform/mac/fast/events/content-inset-hit-testing-in-frame-expected.txt: Copied from platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame-expected.txt.
* platform/mac/fast/events/content-inset-hit-testing-in-frame.html: Copied from platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame.html.
* platform/mac/fast/events/content-inset-hit-testing.html: Copied from platform/mac-wk2/tiled-drawing/content-inset-hit-testing.html.
* platform/mac/fast/events/resources: Added.
* platform/mac/fast/events/resources/iframe-to-hit-test.html: Copied from platform/mac-wk2/tiled-drawing/resources/iframe-to-hit-test.html.

Modified Paths

Added Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (171890 => 171891)


--- trunk/LayoutTests/ChangeLog	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/LayoutTests/ChangeLog	2014-07-31 22:32:40 UTC (rev 171891)
@@ -1,3 +1,28 @@
+2014-07-31  Beth Dakin  <[email protected]>
+
+        Hit-testing broken in WebKit 1 views with AppKit's contentInsets
+        https://bugs.webkit.org/show_bug.cgi?id=135434
+        -and corresponding-
+        <rdar://problem/17850323>
+
+        Reviewed by Benjamin Poulain.
+
+        These hit-testing related contentInset tests will now work for mac on wk1 on 
+        Yosemite and wk2.
+
+        * platform/mac-wk1/TestExpectations:
+        * platform/mac-wk2/tiled-drawing/content-inset-hit-testing-expected.txt: Removed.
+        * platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame-expected.txt: Removed.
+        * platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame.html: Removed.
+        * platform/mac-wk2/tiled-drawing/content-inset-hit-testing.html: Removed.
+        * platform/mac-wk2/tiled-drawing/resources/iframe-to-hit-test.html: Removed.
+        * platform/mac/fast/events/content-inset-hit-testing-expected.txt: Copied from platform/mac-wk2/tiled-drawing/content-inset-hit-testing-expected.txt.
+        * platform/mac/fast/events/content-inset-hit-testing-in-frame-expected.txt: Copied from platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame-expected.txt.
+        * platform/mac/fast/events/content-inset-hit-testing-in-frame.html: Copied from platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame.html.
+        * platform/mac/fast/events/content-inset-hit-testing.html: Copied from platform/mac-wk2/tiled-drawing/content-inset-hit-testing.html.
+        * platform/mac/fast/events/resources: Added.
+        * platform/mac/fast/events/resources/iframe-to-hit-test.html: Copied from platform/mac-wk2/tiled-drawing/resources/iframe-to-hit-test.html.
+
 2014-07-31  Bear Travis  <[email protected]>
 
         [CSS Font Loading] Rename document.fontloader to document.fonts

Copied: trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing-expected.txt (from rev 171873, trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-expected.txt) (0 => 171891)


--- trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing-expected.txt	2014-07-31 22:32:40 UTC (rev 171891)
@@ -0,0 +1 @@
+Pass!

Copied: trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing-in-frame-expected.txt (from rev 171873, trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame-expected.txt) (0 => 171891)


--- trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing-in-frame-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing-in-frame-expected.txt	2014-07-31 22:32:40 UTC (rev 171891)
@@ -0,0 +1,2 @@
+
+Pass!

Copied: trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing-in-frame.html (from rev 171873, trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame.html) (0 => 171891)


--- trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing-in-frame.html	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing-in-frame.html	2014-07-31 22:32:40 UTC (rev 171891)
@@ -0,0 +1,22 @@
+<html>
+<head>
+<script>
+    function runTest() {
+        if (!window.eventSender)
+            return;
+        if (window.internals)
+            window.internals.setTopContentInset(100);
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        eventSender.mouseMoveTo(20, 120);
+        eventSender.mouseDown();
+        eventSender.mouseUp();
+    }
+</script>
+</head>
+<body _onload_="runTest()">
+    <iframe src="" scrolling="no" frameborder="no" height="300" width="600"></iframe>
+    <div id="result">This test needs to be run through WebKitTestRunner.</div>
+</body>
+</html>

Copied: trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing.html (from rev 171873, trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing.html) (0 => 171891)


--- trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing.html	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/events/content-inset-hit-testing.html	2014-07-31 22:32:40 UTC (rev 171891)
@@ -0,0 +1,41 @@
+<html>
+<head>
+<style>
+    #target {
+        background-color:purple;
+        width:25px;
+        height:25px;
+    }
+</style>
+<script>
+    var numberOfClicks = 0;
+
+    function runTest() {
+        if (!window.eventSender)
+            return;
+        if (window.internals)
+            window.internals.setTopContentInset(100);
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        eventSender.mouseMoveTo(15, 115);
+        eventSender.mouseDown();
+        eventSender.mouseUp();
+
+        var result = document.getElementById("result");
+        if (numberOfClicks > 0)
+            result.innerHTML = "Pass!";
+        else
+            result.innerHTML = "Fail.";
+    }
+    
+    function clicked() {
+        numberOfClicks = 1;
+    }
+</script>
+</head>
+<body _onload_="runTest()">
+    <div id="target" _onclick_="clicked()"></div>
+    <div id="result">This test needs to be run through WebKitTestRunner.</div>
+</body>
+</html>

Copied: trunk/LayoutTests/platform/mac/fast/events/resources/iframe-to-hit-test.html (from rev 171873, trunk/LayoutTests/platform/mac-wk2/tiled-drawing/resources/iframe-to-hit-test.html) (0 => 171891)


--- trunk/LayoutTests/platform/mac/fast/events/resources/iframe-to-hit-test.html	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/events/resources/iframe-to-hit-test.html	2014-07-31 22:32:40 UTC (rev 171891)
@@ -0,0 +1,20 @@
+<html>
+<head>
+<style>
+    #target {
+        background-color:purple;
+        width:25px;
+        height:25px;
+    }
+</style>
+<script>
+    function clicked() {
+        var result = parent.document.getElementById("result");
+        result.innerHTML = "Pass!";
+    }
+</script>
+</head>
+<body>
+    <div id="target" _onclick_="clicked()"></div>
+</body>
+</html>

Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (171890 => 171891)


--- trunk/LayoutTests/platform/mac-wk1/TestExpectations	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations	2014-07-31 22:32:40 UTC (rev 171891)
@@ -47,5 +47,9 @@
 # Disable snapshotting tests on WK1 only
 plugins/snapshotting
 
+# Mavericks and prior do not support contentInset
+[ MountainLion Mavericks ] platform/mac/fast/events/content-inset-hit-testing.html [ Skip ]
+[ MountainLion Mavericks ] platform/mac/fast/events/content-inset-hit-testing-in-frame.html [ Skip ]
+
 ### END OF (2) Failures without bug reports
 ########################################

Deleted: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-expected.txt (171890 => 171891)


--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-expected.txt	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-expected.txt	2014-07-31 22:32:40 UTC (rev 171891)
@@ -1 +0,0 @@
-Pass!

Deleted: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame-expected.txt (171890 => 171891)


--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame-expected.txt	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame-expected.txt	2014-07-31 22:32:40 UTC (rev 171891)
@@ -1,2 +0,0 @@
-
-Pass!

Deleted: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame.html (171890 => 171891)


--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame.html	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing-in-frame.html	2014-07-31 22:32:40 UTC (rev 171891)
@@ -1,22 +0,0 @@
-<html>
-<head>
-<script>
-    function runTest() {
-        if (!window.eventSender)
-            return;
-        if (window.internals)
-            window.internals.setTopContentInset(100);
-        if (window.testRunner)
-            testRunner.dumpAsText();
-
-        eventSender.mouseMoveTo(20, 120);
-        eventSender.mouseDown();
-        eventSender.mouseUp();
-    }
-</script>
-</head>
-<body _onload_="runTest()">
-    <iframe src="" scrolling="no" frameborder="no" height="300" width="600"></iframe>
-    <div id="result">This test needs to be run through WebKitTestRunner.</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing.html (171890 => 171891)


--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing.html	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/content-inset-hit-testing.html	2014-07-31 22:32:40 UTC (rev 171891)
@@ -1,41 +0,0 @@
-<html>
-<head>
-<style>
-    #target {
-        background-color:purple;
-        width:25px;
-        height:25px;
-    }
-</style>
-<script>
-    var numberOfClicks = 0;
-
-    function runTest() {
-        if (!window.eventSender)
-            return;
-        if (window.internals)
-            window.internals.setTopContentInset(100);
-        if (window.testRunner)
-            testRunner.dumpAsText();
-
-        eventSender.mouseMoveTo(15, 115);
-        eventSender.mouseDown();
-        eventSender.mouseUp();
-
-        var result = document.getElementById("result");
-        if (numberOfClicks > 0)
-            result.innerHTML = "Pass!";
-        else
-            result.innerHTML = "Fail.";
-    }
-    
-    function clicked() {
-        numberOfClicks = 1;
-    }
-</script>
-</head>
-<body _onload_="runTest()">
-    <div id="target" _onclick_="clicked()"></div>
-    <div id="result">This test needs to be run through WebKitTestRunner.</div>
-</body>
-</html>

Deleted: trunk/LayoutTests/platform/mac-wk2/tiled-drawing/resources/iframe-to-hit-test.html (171890 => 171891)


--- trunk/LayoutTests/platform/mac-wk2/tiled-drawing/resources/iframe-to-hit-test.html	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/LayoutTests/platform/mac-wk2/tiled-drawing/resources/iframe-to-hit-test.html	2014-07-31 22:32:40 UTC (rev 171891)
@@ -1,20 +0,0 @@
-<html>
-<head>
-<style>
-    #target {
-        background-color:purple;
-        width:25px;
-        height:25px;
-    }
-</style>
-<script>
-    function clicked() {
-        var result = parent.document.getElementById("result");
-        result.innerHTML = "Pass!";
-    }
-</script>
-</head>
-<body>
-    <div id="target" _onclick_="clicked()"></div>
-</body>
-</html>

Modified: trunk/Source/WebCore/ChangeLog (171890 => 171891)


--- trunk/Source/WebCore/ChangeLog	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/ChangeLog	2014-07-31 22:32:40 UTC (rev 171891)
@@ -1,3 +1,78 @@
+2014-07-31  Beth Dakin  <[email protected]>
+
+        Hit-testing broken in WebKit 1 views with AppKit's contentInsets
+        https://bugs.webkit.org/show_bug.cgi?id=135434
+        -and corresponding-
+        <rdar://problem/17850323>
+
+        Reviewed by Benjamin Poulain.
+
+        AppKit's contentInsets are factored into scroll positions and mouse positions, but
+        in WebCore, we generally want all of those things to be factored out so that, for
+        example, the scroll position of a document pinned to the top is the same whether
+        or not there is a contentInset. So to fix hit-testing in WebKit 1 views with
+        contentInsets, this patch factors the inset out or into of all coordinate
+        conversion methods just like we do for the WebKit2 contentInset.
+        
+        This patch also adds the ability to test WK1 platformContentInsets with
+        window.internals, and it re-names convertFromRenderer to 
+        convertFromRendererToContainingView and also re-names convertToRenderer to
+        convertFromContainingViewToRenderer.
+
+        ScrollView::topContentInset() takes an optional parameter indicating whether the
+        caller wants the WebCore::Page contentInset or the platform content inset. It’s
+        necessary to distinguish between these cases because there is a lot of code that
+        only wants the WebCore::Page contentInset since that feature is actually
+        implemented in WebCore as opposed to being implemented at the platform level.
+        * WebCore.exp.in:
+        * page/FrameView.cpp:
+        (WebCore::FrameView::topContentInset):
+
+        For layout test purposes, set the platforTopContentInset here if there is a 
+        platformWidget().
+        (WebCore::FrameView::topContentInsetDidChange):
+
+        Re-named functions, also all coordinate conversion functions call 
+        topContentInset(ManualOrPlatformContentInset)
+        (WebCore::FrameView::convertFromContainingViewToRenderer):
+        (WebCore::FrameView::convertToContainingView):
+        (WebCore::FrameView::convertFromContainingView):
+        (WebCore::FrameView::convertFromRenderer): Deleted.
+        (WebCore::FrameView::convertToRenderer): Deleted.
+        * page/FrameView.h:
+        * page/Page.cpp:
+        (WebCore::Page::setTopContentInset):
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::documentScrollOffsetRelativeToViewOrigin):
+        (WebCore::ScrollView::documentScrollPositionRelativeToViewOrigin):
+        (WebCore::ScrollView::contentsToRootView):
+        (WebCore::ScrollView::rootViewToTotalContents):
+        (WebCore::ScrollView::contentsToWindow):
+        (WebCore::ScrollView::platformTopContentInset):
+        (WebCore::ScrollView::platformSetTopContentInset):
+        * platform/ScrollView.h:
+        (WebCore::ScrollView::topContentInset):
+        * platform/ios/ScrollViewIOS.mm:
+        (WebCore::ScrollView::platformTopContentInset):
+        (WebCore::ScrollView::platformSetTopContentInset):
+
+        Implement new platform inset-related functions using AppKit's implementation.
+        * platform/mac/ScrollViewMac.mm:
+        (WebCore::ScrollView::platformTopContentInset):
+        (WebCore::ScrollView::platformSetTopContentInset):
+        
+        When we set the scroll position for the documentView, we have to factor the inset
+        back into the WebCore scroll position.
+        (WebCore::ScrollView::platformSetScrollPosition):
+        
+        Re-named functions.
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::convertFromScrollbarToContainingView):
+        (WebCore::RenderLayer::convertFromContainingViewToScrollbar):
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::convertFromScrollbarToContainingView):
+        (WebCore::RenderListBox::convertFromContainingViewToScrollbar):
+
 2014-07-31  Dean Jackson  <[email protected]>
 
         Default buttons do not pulse in OS X 10.10

Modified: trunk/Source/WebCore/WebCore.exp.in (171890 => 171891)


--- trunk/Source/WebCore/WebCore.exp.in	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/WebCore.exp.in	2014-07-31 22:32:40 UTC (rev 171891)
@@ -2017,18 +2017,18 @@
 __ZNK7WebCore9FrameView13isTransparentEv
 __ZNK7WebCore9FrameView13paintBehaviorEv
 __ZNK7WebCore9FrameView14didFirstLayoutEv
-__ZNK7WebCore9FrameView15topContentInsetEv
-__ZNK7WebCore9FrameView17convertToRendererEPKNS_13RenderElementERKNS_7IntRectE
-__ZNK7WebCore9FrameView17convertToRendererEPKNS_13RenderElementERKNS_8IntPointE
+__ZNK7WebCore9FrameView15topContentInsetENS_10ScrollView19TopContentInsetTypeE
 __ZNK7WebCore9FrameView19baseBackgroundColorEv
-__ZNK7WebCore9FrameView19convertFromRendererEPKNS_13RenderElementERKNS_7IntRectE
-__ZNK7WebCore9FrameView19convertFromRendererEPKNS_13RenderElementERKNS_8IntPointE
 __ZNK7WebCore9FrameView20isSoftwareRenderableEv
 __ZNK7WebCore9FrameView21maximumScrollPositionEv
 __ZNK7WebCore9FrameView21minimumScrollPositionEv
 __ZNK7WebCore9FrameView23documentBackgroundColorEv
 __ZNK7WebCore9FrameView27windowClipRectForFrameOwnerEPKNS_21HTMLFrameOwnerElementEb
 __ZNK7WebCore9FrameView28isEnclosedInCompositingLayerEv
+__ZNK7WebCore9FrameView35convertFromContainingViewToRendererEPKNS_13RenderElementERKNS_7IntRectE
+__ZNK7WebCore9FrameView35convertFromContainingViewToRendererEPKNS_13RenderElementERKNS_8IntPointE
+__ZNK7WebCore9FrameView35convertFromRendererToContainingViewEPKNS_13RenderElementERKNS_7IntRectE
+__ZNK7WebCore9FrameView35convertFromRendererToContainingViewEPKNS_13RenderElementERKNS_8IntPointE
 __ZNK7WebCore9InlineBox10lineHeightEv
 __ZNK7WebCore9InlineBox14caretMaxOffsetEv
 __ZNK7WebCore9InlineBox14caretMinOffsetEv

Modified: trunk/Source/WebCore/page/FrameView.cpp (171890 => 171891)


--- trunk/Source/WebCore/page/FrameView.cpp	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/page/FrameView.cpp	2014-07-31 22:32:40 UTC (rev 171891)
@@ -930,8 +930,11 @@
         renderView->setNeedsLayout();
 }
 
-float FrameView::topContentInset() const
+float FrameView::topContentInset(TopContentInsetType contentInsetTypeToReturn) const
 {
+    if (platformWidget() && contentInsetTypeToReturn == TopContentInsetType::WebCoreOrPlatformContentInset)
+        return platformTopContentInset();
+
     if (!frame().isMainFrame())
         return 0;
     
@@ -939,11 +942,14 @@
     return page ? page->topContentInset() : 0;
 }
     
-void FrameView::topContentInsetDidChange()
+void FrameView::topContentInsetDidChange(float newTopContentInset)
 {
     RenderView* renderView = this->renderView();
     if (!renderView)
         return;
+
+    if (platformWidget())
+        platformSetTopContentInset(newTopContentInset);
     
     layout();
 
@@ -952,7 +958,7 @@
         renderView->compositor().frameViewDidChangeSize();
 
     if (TiledBacking* tiledBacking = this->tiledBacking())
-        tiledBacking->setTopContentInset(topContentInset());
+        tiledBacking->setTopContentInset(newTopContentInset);
 }
     
 bool FrameView::hasCompositedContent() const
@@ -3931,18 +3937,18 @@
     renderView->setPrintRect(IntRect());
 }
 
-IntRect FrameView::convertFromRenderer(const RenderElement* renderer, const IntRect& rendererRect) const
+IntRect FrameView::convertFromRendererToContainingView(const RenderElement* renderer, const IntRect& rendererRect) const
 {
     IntRect rect = pixelSnappedIntRect(enclosingLayoutRect(renderer->localToAbsoluteQuad(FloatRect(rendererRect)).boundingBox()));
 
     // Convert from page ("absolute") to FrameView coordinates.
     if (!delegatesScrolling())
-        rect.moveBy(-scrollPosition() + IntPoint(0, headerHeight() + topContentInset()));
+        rect.moveBy(-scrollPosition() + IntPoint(0, headerHeight() + topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset)));
 
     return rect;
 }
 
-IntRect FrameView::convertToRenderer(const RenderElement* renderer, const IntRect& viewRect) const
+IntRect FrameView::convertFromContainingViewToRenderer(const RenderElement* renderer, const IntRect& viewRect) const
 {
     IntRect rect = viewRect;
     
@@ -3956,17 +3962,17 @@
     return rect;
 }
 
-IntPoint FrameView::convertFromRenderer(const RenderElement* renderer, const IntPoint& rendererPoint) const
+IntPoint FrameView::convertFromRendererToContainingView(const RenderElement* renderer, const IntPoint& rendererPoint) const
 {
     IntPoint point = roundedIntPoint(renderer->localToAbsolute(rendererPoint, UseTransforms));
 
     // Convert from page ("absolute") to FrameView coordinates.
     if (!delegatesScrolling())
-        point.moveBy(-scrollPosition() + IntPoint(0, headerHeight() + topContentInset()));
+        point.moveBy(-scrollPosition() + IntPoint(0, headerHeight() + topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset)));
     return point;
 }
 
-IntPoint FrameView::convertToRenderer(const RenderElement* renderer, const IntPoint& viewPoint) const
+IntPoint FrameView::convertFromContainingViewToRenderer(const RenderElement* renderer, const IntPoint& viewPoint) const
 {
     IntPoint point = viewPoint;
 
@@ -3991,7 +3997,7 @@
             // Add borders and padding??
             rect.move(renderer->borderLeft() + renderer->paddingLeft(),
                       renderer->borderTop() + renderer->paddingTop());
-            return parentView->convertFromRenderer(renderer, rect);
+            return parentView->convertFromRendererToContainingView(renderer, rect);
         }
         
         return Widget::convertToContainingView(localRect);
@@ -4011,7 +4017,7 @@
             if (!renderer)
                 return parentRect;
 
-            IntRect rect = parentView->convertToRenderer(renderer, parentRect);
+            IntRect rect = parentView->convertFromContainingViewToRenderer(renderer, parentRect);
             // Subtract borders and padding
             rect.move(-renderer->borderLeft() - renderer->paddingLeft(),
                       -renderer->borderTop() - renderer->paddingTop());
@@ -4040,7 +4046,7 @@
             // Add borders and padding
             point.move(renderer->borderLeft() + renderer->paddingLeft(),
                        renderer->borderTop() + renderer->paddingTop());
-            return parentView->convertFromRenderer(renderer, point);
+            return parentView->convertFromRendererToContainingView(renderer, point);
         }
         
         return Widget::convertToContainingView(localPoint);
@@ -4060,7 +4066,7 @@
             if (!renderer)
                 return parentPoint;
 
-            IntPoint point = parentView->convertToRenderer(renderer, parentPoint);
+            IntPoint point = parentView->convertFromContainingViewToRenderer(renderer, parentPoint);
             // Subtract borders and padding
             point.move(-renderer->borderLeft() - renderer->paddingLeft(),
                        -renderer->borderTop() - renderer->paddingTop());

Modified: trunk/Source/WebCore/page/FrameView.h (171890 => 171891)


--- trunk/Source/WebCore/page/FrameView.h	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/page/FrameView.h	2014-07-31 22:32:40 UTC (rev 171891)
@@ -387,10 +387,10 @@
     void scrollElementToRect(Element*, const IntRect&);
 
     // Methods to convert points and rects between the coordinate space of the renderer, and this view.
-    IntRect convertFromRenderer(const RenderElement*, const IntRect&) const;
-    IntRect convertToRenderer(const RenderElement*, const IntRect&) const;
-    IntPoint convertFromRenderer(const RenderElement*, const IntPoint&) const;
-    IntPoint convertToRenderer(const RenderElement*, const IntPoint&) const;
+    IntRect convertFromRendererToContainingView(const RenderElement*, const IntRect&) const;
+    IntRect convertFromContainingViewToRenderer(const RenderElement*, const IntRect&) const;
+    IntPoint convertFromRendererToContainingView(const RenderElement*, const IntPoint&) const;
+    IntPoint convertFromContainingViewToRenderer(const RenderElement*, const IntPoint&) const;
 
     bool isFrameViewScrollCorner(RenderScrollbarPart* scrollCorner) const { return m_scrollCorner == scrollCorner; }
 
@@ -465,8 +465,8 @@
     virtual int footerHeight() const override { return m_footerHeight; }
     void setFooterHeight(int);
 
-    virtual float topContentInset() const override;
-    void topContentInsetDidChange();
+    virtual float topContentInset(TopContentInsetType = TopContentInsetType::WebCoreContentInset) const override;
+    void topContentInsetDidChange(float newTopContentInset);
 
     virtual void willStartLiveResize() override;
     virtual void willEndLiveResize() override;

Modified: trunk/Source/WebCore/page/Page.cpp (171890 => 171891)


--- trunk/Source/WebCore/page/Page.cpp	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/page/Page.cpp	2014-07-31 22:32:40 UTC (rev 171891)
@@ -806,7 +806,7 @@
     m_topContentInset = contentInset;
     
     if (FrameView* view = mainFrame().view())
-        view->topContentInsetDidChange();
+        view->topContentInsetDidChange(m_topContentInset);
 }
 
 void Page::setShouldSuppressScrollbarAnimations(bool suppressAnimations)

Modified: trunk/Source/WebCore/platform/ScrollView.cpp (171890 => 171891)


--- trunk/Source/WebCore/platform/ScrollView.cpp	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/platform/ScrollView.cpp	2014-07-31 22:32:40 UTC (rev 171891)
@@ -410,13 +410,13 @@
 
 IntSize ScrollView::documentScrollOffsetRelativeToViewOrigin() const
 {
-    return scrollOffset() - IntSize(0, headerHeight() + topContentInset());
+    return scrollOffset() - IntSize(0, headerHeight() + topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset));
 }
 
 IntPoint ScrollView::documentScrollPositionRelativeToViewOrigin() const
 {
     IntPoint scrollPosition = this->scrollPosition();
-    return IntPoint(scrollPosition.x(), scrollPosition.y() - headerHeight() - topContentInset());
+    return IntPoint(scrollPosition.x(), scrollPosition.y() - headerHeight() - topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset));
 }
 
 IntSize ScrollView::documentScrollOffsetRelativeToScrollableAreaOrigin() const
@@ -830,7 +830,7 @@
     if (delegatesScrolling())
         return convertToRootView(contentsPoint);
 
-    IntPoint viewPoint = contentsPoint + IntSize(0, headerHeight() + topContentInset()) - scrollOffset();
+    IntPoint viewPoint = contentsPoint + IntSize(0, headerHeight() + topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset)) - scrollOffset();
     return convertToRootView(viewPoint);  
 }
 
@@ -850,7 +850,7 @@
         return convertToRootView(contentsRect);
 
     IntRect viewRect = contentsRect;
-    viewRect.move(-scrollOffset() + IntSize(0, headerHeight() + topContentInset()));
+    viewRect.move(-scrollOffset() + IntSize(0, headerHeight() + topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset)));
     return convertToRootView(viewRect);
 }
 
@@ -860,7 +860,7 @@
         return convertFromRootView(rootViewPoint);
 
     IntPoint viewPoint = convertFromRootView(rootViewPoint);
-    return viewPoint + scrollOffset() - IntSize(0, topContentInset());
+    return viewPoint + scrollOffset() - IntSize(0, topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset));
 }
 
 IntPoint ScrollView::windowToContents(const IntPoint& windowPoint) const
@@ -877,7 +877,7 @@
     if (delegatesScrolling())
         return convertToContainingWindow(contentsPoint);
 
-    IntPoint viewPoint = contentsPoint + IntSize(0, headerHeight() + topContentInset()) - scrollOffset();
+    IntPoint viewPoint = contentsPoint + IntSize(0, headerHeight() + topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset)) - scrollOffset();
     return convertToContainingWindow(viewPoint);  
 }
 
@@ -897,7 +897,7 @@
         return convertToContainingWindow(contentsRect);
 
     IntRect viewRect = contentsRect;
-    viewRect.move(-scrollOffset() + IntSize(0, headerHeight() + topContentInset()));
+    viewRect.move(-scrollOffset() + IntSize(0, headerHeight() + topContentInset(TopContentInsetType::WebCoreOrPlatformContentInset)));
     return convertToContainingWindow(viewRect);
 }
 
@@ -1515,6 +1515,15 @@
     return IntRect();
 }
 
+float ScrollView::platformTopContentInset() const
+{
+    return 0;
+}
+
+void ScrollView::platformSetTopContentInset(float)
+{
+}
+
 IntSize ScrollView::platformVisibleContentSize(bool) const
 {
     return IntSize();

Modified: trunk/Source/WebCore/platform/ScrollView.h (171890 => 171891)


--- trunk/Source/WebCore/platform/ScrollView.h	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/platform/ScrollView.h	2014-07-31 22:32:40 UTC (rev 171891)
@@ -156,7 +156,13 @@
     void setCanBlitOnScroll(bool);
     bool canBlitOnScroll() const;
 
-    virtual float topContentInset() const { return 0; }
+    // There are at least three types of contentInset. Usually we just care about WebCoreContentInset, which is the inset
+    // that is set on a Page that requires WebCore to move its layers to accomodate the inset. However, there are platform
+    // concepts that are similar on both iOS and Mac when there is a platformWidget(). Sometimes we need the Mac platform value
+    // for topContentInset, so when the TopContentInsetType is WebCoreOrPlatformContentInset, platformTopContentInset()
+    // will be returned instead of the value set on Page.
+    enum class TopContentInsetType { WebCoreContentInset, WebCoreOrPlatformContentInset };
+    virtual float topContentInset(TopContentInsetType = TopContentInsetType::WebCoreContentInset) const { return 0; }
 
     // The visible content rect has a location that is the scrolled offset of the document. The width and height are the unobscured viewport
     // width and height. By default the scrollbars themselves are excluded from this rectangle, but an optional boolean argument allows them
@@ -408,6 +414,9 @@
     // Called to update the scrollbars to accurately reflect the state of the view.
     void updateScrollbars(const IntSize& desiredOffset);
 
+    float platformTopContentInset() const;
+    void platformSetTopContentInset(float);
+
 private:
     virtual IntRect visibleContentRectInternal(VisibleContentRectIncludesScrollbars, VisibleContentRectBehavior) const override;
     IntRect unobscuredContentRectInternal(VisibleContentRectIncludesScrollbars = ExcludeScrollbars) const;

Modified: trunk/Source/WebCore/platform/ios/ScrollViewIOS.mm (171890 => 171891)


--- trunk/Source/WebCore/platform/ios/ScrollViewIOS.mm	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/platform/ios/ScrollViewIOS.mm	2014-07-31 22:32:40 UTC (rev 171891)
@@ -161,6 +161,15 @@
     END_BLOCK_OBJC_EXCEPTIONS;
 }
 
+float ScrollView::platformTopContentInset(float) const
+{
+    return 0;
+}
+
+void ScrollView::platformSetTopContentInset()
+{
+}
+
 IntRect ScrollView::platformVisibleContentRect(bool includeScrollbars) const
 {
     BEGIN_BLOCK_OBJC_EXCEPTIONS;

Modified: trunk/Source/WebCore/platform/mac/ScrollViewMac.mm (171890 => 171891)


--- trunk/Source/WebCore/platform/mac/ScrollViewMac.mm	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/platform/mac/ScrollViewMac.mm	2014-07-31 22:32:40 UTC (rev 171891)
@@ -108,6 +108,35 @@
     return [[scrollView() contentView] copiesOnScroll];
 }
 
+float ScrollView::platformTopContentInset() const
+{
+    BEGIN_BLOCK_OBJC_EXCEPTIONS;
+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
+    return scrollView().contentInsets.top;
+#endif
+    END_BLOCK_OBJC_EXCEPTIONS;
+
+    return 0;
+}
+
+void ScrollView::platformSetTopContentInset(float topContentInset)
+{
+    BEGIN_BLOCK_OBJC_EXCEPTIONS;
+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
+    if (topContentInset)
+        scrollView().automaticallyAdjustsContentInsets = NO;
+    else
+        scrollView().automaticallyAdjustsContentInsets = YES;
+
+    NSEdgeInsets contentInsets = scrollView().contentInsets;
+    contentInsets.top = topContentInset;
+    scrollView().contentInsets = contentInsets;
+#else
+    UNUSED_PARAM(topContentInset);
+#endif
+    END_BLOCK_OBJC_EXCEPTIONS;
+}
+
 IntRect ScrollView::platformVisibleContentRect(bool includeScrollbars) const
 {
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
@@ -160,6 +189,14 @@
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
     NSPoint floatPoint = scrollPoint;
     NSPoint tempPoint = { std::max(-[scrollView() scrollOrigin].x, floatPoint.x), std::max(-[scrollView() scrollOrigin].y, floatPoint.y) };  // Don't use NSMakePoint to work around 4213314.
+
+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
+    // AppKit has the inset factored into all of its scroll positions. In WebCore, we use positions that ignore
+    // the insets so that they are equivalent whether or not there is an inset.
+    tempPoint.x = tempPoint.x - scrollView().contentInsets.left;
+    tempPoint.y = tempPoint.y - scrollView().contentInsets.top;
+#endif
+
     [documentView() scrollPoint:tempPoint];
     END_BLOCK_OBJC_EXCEPTIONS;
 }

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (171890 => 171891)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2014-07-31 22:32:40 UTC (rev 171891)
@@ -2730,12 +2730,12 @@
     IntRect rect = scrollbarRect;
     rect.move(scrollbarOffset(scrollbar));
 
-    return renderer().view().frameView().convertFromRenderer(&renderer(), rect);
+    return renderer().view().frameView().convertFromRendererToContainingView(&renderer(), rect);
 }
 
 IntRect RenderLayer::convertFromContainingViewToScrollbar(const Scrollbar* scrollbar, const IntRect& parentRect) const
 {
-    IntRect rect = renderer().view().frameView().convertToRenderer(&renderer(), parentRect);
+    IntRect rect = renderer().view().frameView().convertFromContainingViewToRenderer(&renderer(), parentRect);
     rect.move(-scrollbarOffset(scrollbar));
     return rect;
 }
@@ -2744,12 +2744,12 @@
 {
     IntPoint point = scrollbarPoint;
     point.move(scrollbarOffset(scrollbar));
-    return renderer().view().frameView().convertFromRenderer(&renderer(), point);
+    return renderer().view().frameView().convertFromRendererToContainingView(&renderer(), point);
 }
 
 IntPoint RenderLayer::convertFromContainingViewToScrollbar(const Scrollbar* scrollbar, const IntPoint& parentPoint) const
 {
-    IntPoint point = renderer().view().frameView().convertToRenderer(&renderer(), parentPoint);
+    IntPoint point = renderer().view().frameView().convertFromContainingViewToRenderer(&renderer(), parentPoint);
     point.move(-scrollbarOffset(scrollbar));
     return point;
 }

Modified: trunk/Source/WebCore/rendering/RenderListBox.cpp (171890 => 171891)


--- trunk/Source/WebCore/rendering/RenderListBox.cpp	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebCore/rendering/RenderListBox.cpp	2014-07-31 22:32:40 UTC (rev 171891)
@@ -725,12 +725,12 @@
     int scrollbarLeft = width() - borderRight() - scrollbar->width();
     int scrollbarTop = borderTop();
     rect.move(scrollbarLeft, scrollbarTop);
-    return view().frameView().convertFromRenderer(this, rect);
+    return view().frameView().convertFromRendererToContainingView(this, rect);
 }
 
 IntRect RenderListBox::convertFromContainingViewToScrollbar(const Scrollbar* scrollbar, const IntRect& parentRect) const
 {
-    IntRect rect = view().frameView().convertToRenderer(this, parentRect);
+    IntRect rect = view().frameView().convertFromContainingViewToRenderer(this, parentRect);
     int scrollbarLeft = width() - borderRight() - scrollbar->width();
     int scrollbarTop = borderTop();
     rect.move(-scrollbarLeft, -scrollbarTop);
@@ -743,12 +743,12 @@
     int scrollbarLeft = width() - borderRight() - scrollbar->width();
     int scrollbarTop = borderTop();
     point.move(scrollbarLeft, scrollbarTop);
-    return view().frameView().convertFromRenderer(this, point);
+    return view().frameView().convertFromRendererToContainingView(this, point);
 }
 
 IntPoint RenderListBox::convertFromContainingViewToScrollbar(const Scrollbar* scrollbar, const IntPoint& parentPoint) const
 {
-    IntPoint point = view().frameView().convertToRenderer(this, parentPoint);
+    IntPoint point = view().frameView().convertFromContainingViewToRenderer(this, parentPoint);
     int scrollbarLeft = width() - borderRight() - scrollbar->width();
     int scrollbarTop = borderTop();
     point.move(-scrollbarLeft, -scrollbarTop);

Modified: trunk/Source/WebKit2/ChangeLog (171890 => 171891)


--- trunk/Source/WebKit2/ChangeLog	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebKit2/ChangeLog	2014-07-31 22:32:40 UTC (rev 171891)
@@ -1,3 +1,17 @@
+2014-07-31  Beth Dakin  <[email protected]>
+
+        Hit-testing broken in WebKit 1 views with AppKit's contentInsets
+        https://bugs.webkit.org/show_bug.cgi?id=135434
+        -and corresponding-
+        <rdar://problem/17850323>
+
+        Reviewed by Benjamin Poulain.
+
+        Re-named functions.
+        * WebProcess/Plugins/PDF/PDFPlugin.mm:
+        (WebKit::PDFPlugin::convertFromScrollbarToContainingView):
+        (WebKit::PDFPlugin::convertFromContainingViewToScrollbar):
+
 2014-07-31  Oliver Hunt  <[email protected]>
 
         Various SSO services need the networking process to be able to spoof its bundle id

Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm (171890 => 171891)


--- trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm	2014-07-31 22:07:46 UTC (rev 171890)
+++ trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm	2014-07-31 22:32:40 UTC (rev 171891)
@@ -679,12 +679,12 @@
     IntRect rect = scrollbarRect;
     rect.move(scrollbar->location() - pluginView()->location());
 
-    return pluginView()->frame()->view()->convertFromRenderer(pluginView()->renderer(), rect);
+    return pluginView()->frame()->view()->convertFromRendererToContainingView(pluginView()->renderer(), rect);
 }
 
 IntRect PDFPlugin::convertFromContainingViewToScrollbar(const Scrollbar* scrollbar, const IntRect& parentRect) const
 {
-    IntRect rect = pluginView()->frame()->view()->convertToRenderer(pluginView()->renderer(), parentRect);
+    IntRect rect = pluginView()->frame()->view()->convertFromContainingViewToRenderer(pluginView()->renderer(), parentRect);
     rect.move(pluginView()->location() - scrollbar->location());
 
     return rect;
@@ -695,12 +695,12 @@
     IntPoint point = scrollbarPoint;
     point.move(scrollbar->location() - pluginView()->location());
 
-    return pluginView()->frame()->view()->convertFromRenderer(pluginView()->renderer(), point);
+    return pluginView()->frame()->view()->convertFromRendererToContainingView(pluginView()->renderer(), point);
 }
 
 IntPoint PDFPlugin::convertFromContainingViewToScrollbar(const Scrollbar* scrollbar, const IntPoint& parentPoint) const
 {
-    IntPoint point = pluginView()->frame()->view()->convertToRenderer(pluginView()->renderer(), parentPoint);
+    IntPoint point = pluginView()->frame()->view()->convertFromContainingViewToRenderer(pluginView()->renderer(), parentPoint);
     point.move(pluginView()->location() - scrollbar->location());
     
     return point;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to