Title: [152433] trunk
Revision
152433
Author
timothy_hor...@apple.com
Date
2013-07-05 19:19:41 -0700 (Fri, 05 Jul 2013)

Log Message

Fix r152265: FrameView's pagination mode is only one of two, and the logic was totally wrong
https://bugs.webkit.org/show_bug.cgi?id=118439
<rdar://problem/14366120>

Reviewed by Anders Carlsson.

* page/FrameView.cpp:
(WebCore::FrameView::maximumScrollPosition):
- Test both Page and FrameView's pagination modes.
- *Don't* clamp negatives to zero if we are paginated with scroll offset set,
    like the original changelog claimed, but the code did the opposite.
- Don't clamp in *all* pagination modes; with 'direction: RTL' it's still possible
    to have a negative maximum position in LTR/TB pagination modes.

* TestWebKitAPI/Tests/WebKit2/ResizeReversePaginatedWebView.cpp:
(TestWebKitAPI::didLayout):
(TestWebKitAPI::TEST):
- Make use of EXPECT_JS_EQ instead of manually doing _javascript_ stuff.
- Assert that we got the right number of pages for sanity.
- Reduce the page gap size so that DrawingAreaImpl doesn't try to allocate
    so much memory that SharedMemory asserts and makes the test time out.
- Use didFirstLayoutAfterSuppressedIncrementalRendering instead of Paint
    because paint doesn't fire if the window is offscreen.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (152432 => 152433)


--- trunk/Source/WebCore/ChangeLog	2013-07-06 01:08:03 UTC (rev 152432)
+++ trunk/Source/WebCore/ChangeLog	2013-07-06 02:19:41 UTC (rev 152433)
@@ -1,3 +1,19 @@
+2013-07-05  Tim Horton  <timothy_hor...@apple.com>
+
+        Fix r152265: FrameView's pagination mode is only one of two, and the logic was totally wrong
+        https://bugs.webkit.org/show_bug.cgi?id=118439
+        <rdar://problem/14366120>
+
+        Reviewed by Anders Carlsson.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::maximumScrollPosition):
+        - Test both Page and FrameView's pagination modes.
+        - *Don't* clamp negatives to zero if we are paginated with scroll offset set,
+            like the original changelog claimed, but the code did the opposite.
+        - Don't clamp in *all* pagination modes; with 'direction: RTL' it's still possible
+            to have a negative maximum position in LTR/TB pagination modes.
+
 2013-07-05  Sam Weinig  <s...@webkit.org>
 
         WKPageFindStringMatches with kWKFindOptionsBackwards still doesn't work

Modified: trunk/Source/WebCore/page/FrameView.cpp (152432 => 152433)


--- trunk/Source/WebCore/page/FrameView.cpp	2013-07-06 01:08:03 UTC (rev 152432)
+++ trunk/Source/WebCore/page/FrameView.cpp	2013-07-06 02:19:41 UTC (rev 152433)
@@ -1653,9 +1653,8 @@
 {
     IntPoint maximumOffset(contentsWidth() - visibleWidth() - scrollOrigin().x(), totalContentsSize().height() - visibleHeight() - scrollOrigin().y());
 
-    // With reverse pagination modes, we can have a negative maximum scroll position.
-    if (m_pagination.mode == Pagination::BottomToTopPaginated
-        || m_pagination.mode == Pagination::RightToLeftPaginated
+    // With pagination enabled, we can have a negative maximum scroll position.
+    if ((m_frame->page()->pagination().mode == Pagination::Unpaginated && m_pagination.mode == Pagination::Unpaginated)
         || scrollOrigin() == IntPoint::zero())
         maximumOffset.clampNegativeToZero();
     

Modified: trunk/Tools/ChangeLog (152432 => 152433)


--- trunk/Tools/ChangeLog	2013-07-06 01:08:03 UTC (rev 152432)
+++ trunk/Tools/ChangeLog	2013-07-06 02:19:41 UTC (rev 152433)
@@ -1,5 +1,23 @@
 2013-07-05  Tim Horton  <timothy_hor...@apple.com>
 
+        Fix r152265: FrameView's pagination mode is only one of two, and the logic was totally wrong
+        https://bugs.webkit.org/show_bug.cgi?id=118439
+        <rdar://problem/14366120>
+
+        Reviewed by Anders Carlsson.
+
+        * TestWebKitAPI/Tests/WebKit2/ResizeReversePaginatedWebView.cpp:
+        (TestWebKitAPI::didLayout):
+        (TestWebKitAPI::TEST):
+        - Make use of EXPECT_JS_EQ instead of manually doing _javascript_ stuff.
+        - Assert that we got the right number of pages for sanity.
+        - Reduce the page gap size so that DrawingAreaImpl doesn't try to allocate
+            so much memory that SharedMemory asserts and makes the test time out.
+        - Use didFirstLayoutAfterSuppressedIncrementalRendering instead of Paint
+            because paint doesn't fire if the window is offscreen.
+
+2013-07-05  Tim Horton  <timothy_hor...@apple.com>
+
         [wk2] Add API to lock the scroll position at the top or bottom of the page
         https://bugs.webkit.org/show_bug.cgi?id=118429
         <rdar://problem/14120323>

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2/ResizeReversePaginatedWebView.cpp (152432 => 152433)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit2/ResizeReversePaginatedWebView.cpp	2013-07-06 01:08:03 UTC (rev 152432)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2/ResizeReversePaginatedWebView.cpp	2013-07-06 02:19:41 UTC (rev 152433)
@@ -24,6 +24,7 @@
  */
 
 #include "config.h"
+#include "_javascript_Test.h"
 #include "PlatformUtilities.h"
 #include "PlatformWebView.h"
 #include "Test.h"
@@ -37,31 +38,21 @@
 static bool testDone;
 
 static const unsigned pageLength = 100;
-static const unsigned pageGap = 1000;
+static const unsigned pageGap = 100;
+static const unsigned expectedPageCount = 20;
 
-static void didRunJavaScript(WKSerializedScriptValueRef resultSerializedScriptValue, WKErrorRef error, void* context)
-{
-    JSGlobalContextRef scriptContext = JSGlobalContextCreate(0);
-    ASSERT_NOT_NULL(scriptContext);
-
-    ASSERT_NOT_NULL(resultSerializedScriptValue);
-    JSValueRef returnValue = WKSerializedScriptValueDeserialize(resultSerializedScriptValue, scriptContext, 0);
-    double scrollPosition = JSValueToNumber(scriptContext, returnValue, 0);
-
-    ASSERT_EQ(-20900.0, scrollPosition);
-
-    testDone = true;
-}
-
 static void didLayout(WKPageRef page, WKLayoutMilestones milestones, WKTypeRef, const void* clientInfo)
 {
-    if (milestones & kWKDidFirstPaintAfterSuppressedIncrementalRendering) {
+    if (milestones & kWKDidFirstLayoutAfterSuppressedIncrementalRendering) {
         PlatformWebView* webView = (PlatformWebView*)clientInfo;
+
         unsigned pageCount = WKPageGetPageCount(page);
+        EXPECT_EQ(expectedPageCount, pageCount);
+
         webView->resizeTo((pageLength * pageCount) + (pageGap * (pageCount - 1)), 500);
+        EXPECT_JS_EQ(page, "window.scrollX", "-3800");
 
-        WKRetainPtr<WKStringRef> _javascript_String(AdoptWK, WKStringCreateWithUTF8CString("window.scrollX"));
-        WKPageRunJavaScriptInMainFrame(page, _javascript_String.get(), 0, didRunJavaScript);
+        testDone = true;
     }
 }
 
@@ -78,7 +69,7 @@
 
     WKPageSetPageLoaderClient(webView.page(), &loaderClient);
 
-    WKPageListenForLayoutMilestones(webView.page(), kWKDidFirstPaintAfterSuppressedIncrementalRendering);
+    WKPageListenForLayoutMilestones(webView.page(), kWKDidFirstLayoutAfterSuppressedIncrementalRendering);
 
     WKPageGroupRef pageGroup =  WKPageGetPageGroup(webView.page());
     WKPreferencesRef preferences = WKPageGroupGetPreferences(pageGroup);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to