[BlackBerry][Fullscreen] Exiting fullscreen does not set the correct scroll position
authorzhajiang@rim.com <zhajiang@rim.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2012 15:33:11 +0000 (15:33 +0000)
committerzhajiang@rim.com <zhajiang@rim.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Dec 2012 15:33:11 +0000 (15:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=104958

Patch by Jacky Jiang  <zhajiang@rim.com>.
Reviewed by Antonio Gomes.
Internally reviewed by Arvid Nilsson and Max Feil.

PR: 231174
When leaving fullscreen, WebPage scale and scroll position can't return
to the original scale and scroll position.
We can't restore them in WebPagePrivate::exitFullScreenForElement()
as they can still be changed thereafter during the async
setViewportSize(). And also the async setViewportSize() from the app
side isn't guaranteed as some apps don't need to resize the viewport if
their windows are already fullscreen.
The restoration is basically only needed if viewport size is changed.
At the point of entering fullscreen, we can safely assume that there
would be a viewport size change thereafter if the current visible size
and screen size are not equal. Based on this assumption, we can save
the scale and position before entering fullscreen and restore them in
setViewportSize() thereafter.

* Api/WebPage.cpp:
(BlackBerry::WebKit::WebPagePrivate::WebPagePrivate):
(BlackBerry::WebKit::WebPagePrivate::setViewportSize):
* Api/WebPage_p.h:
(WebPagePrivate):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@137743 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/blackberry/Api/WebPage.cpp
Source/WebKit/blackberry/Api/WebPage_p.h
Source/WebKit/blackberry/ChangeLog

index 683242e..ab0d097 100644 (file)
@@ -399,7 +399,6 @@ WebPagePrivate::WebPagePrivate(WebPage* webPage, WebPageClient* client, const In
 #endif
 #if ENABLE(FULLSCREEN_API) && ENABLE(VIDEO)
     , m_scaleBeforeFullScreen(-1.0)
-    , m_xScrollOffsetBeforeFullScreen(-1)
 #endif
     , m_currentCursor(Platform::CursorNone)
     , m_dumpRenderTree(0) // Lazy initialization.
@@ -3839,6 +3838,25 @@ void WebPagePrivate::setViewportSize(const IntSize& transformedActualVisibleSize
         m_backingStore->d->resumeBackingStoreUpdates();
     }
 
+#if ENABLE(FULLSCREEN_API) && ENABLE(VIDEO)
+    // When leaving fullscreen mode, restore the scale and scroll position
+    // if needed.
+    // FIXME: The cached values might get imprecise if user have rotated the
+    // device while in fullscreen.
+    if (m_scaleBeforeFullScreen > 0 && !m_fullscreenVideoNode) {
+        // Restore the scale when leaving fullscreen. We can't use TransformationMatrix::scale(double) here,
+        // as it will multiply the scale rather than set the scale.
+        // FIXME: We can refactor this into setCurrentScale(double) if it is useful in the future.
+        m_transformationMatrix->setM11(m_scaleBeforeFullScreen);
+        m_transformationMatrix->setM22(m_scaleBeforeFullScreen);
+        m_scaleBeforeFullScreen = -1.0;
+
+        setScrollPosition(m_scrollPositionBeforeFullScreen);
+        notifyTransformChanged();
+        m_client->scaleChanged();
+    }
+#endif
+
     m_backingStore->d->resumeScreenUpdates(screenResumeOperation);
     m_inputHandler->redrawSpellCheckDialogIfRequired();
 }
@@ -5798,16 +5816,19 @@ void WebPagePrivate::enterFullScreenForElement(Element* element)
         // is so that exitFullScreenForElement() gets called later.
         enterFullscreenForNode(element);
     } else {
-        // When an element goes fullscreen, the viewport size changes and the scroll
-        // position might change. So we keep track of it here, in order to restore it
-        // once element leaves fullscreen.
-        WebCore::IntPoint scrollPosition = m_mainFrame->view()->scrollPosition();
-        m_xScrollOffsetBeforeFullScreen = scrollPosition.x();
-
-        // The current scale can be clamped to a greater minimum scale when we relayout contents during
-        // the change of the viewport size. Cache the current scale so that we can restore it when
-        // leaving fullscreen. Otherwise, it is possible that we will use the wrong scale.
-        m_scaleBeforeFullScreen = currentScale();
+        // At this point, we can assume that there would be a viewport size change if
+        // the current visible size and screen size are not equal.
+        if (transformedActualVisibleSize() != transformedViewportSize()) {
+            // The current scale can be clamped to a greater minimum scale when we relayout contents during
+            // the change of the viewport size. Cache the current scale so that we can restore it when
+            // leaving fullscreen. Otherwise, it is possible that we will use the wrong scale.
+            m_scaleBeforeFullScreen = currentScale();
+
+            // When an element goes fullscreen, the viewport size changes and the scroll
+            // position might change. So we keep track of it here, in order to restore it
+            // once element leaves fullscreen.
+            m_scrollPositionBeforeFullScreen = m_mainFrame->view()->scrollPosition();
+        }
 
         // No fullscreen video widget has been made available by the Browser
         // chrome, or this is not a video element. The webkitRequestFullScreen
@@ -5829,33 +5850,6 @@ void WebPagePrivate::exitFullScreenForElement(Element* element)
         // The Browser chrome has its own fullscreen video widget.
         exitFullscreenForNode(element);
     } else {
-        // FIXME: Do we really need to suspend/resume both backingstore and screen here?
-        m_backingStore->d->suspendBackingStoreUpdates();
-        m_backingStore->d->suspendScreenUpdates();
-
-        // When leaving fullscreen mode, we need to restore the 'x' scroll position
-        // before fullscreen.
-        // FIXME: We may need to respect 'y' position as well, because the web page always scrolls to
-        // the top when leaving fullscreen mode.
-        WebCore::IntPoint scrollPosition = m_mainFrame->view()->scrollPosition();
-        m_mainFrame->view()->setScrollPosition(
-            WebCore::IntPoint(m_xScrollOffsetBeforeFullScreen, scrollPosition.y()));
-        m_xScrollOffsetBeforeFullScreen = -1;
-
-        if (m_scaleBeforeFullScreen > 0) {
-            // Restore the scale when leaving fullscreen. We can't use TransformationMatrix::scale(double) here, as it
-            // will multiply the scale rather than set the scale.
-            // FIXME: We can refactor this into setCurrentScale(double) if it is useful in the future.
-            m_transformationMatrix->setM11(m_scaleBeforeFullScreen);
-            m_transformationMatrix->setM22(m_scaleBeforeFullScreen);
-            m_scaleBeforeFullScreen = -1.0;
-        }
-
-        notifyTransformChanged();
-        // FIXME: Do we really need to suspend/resume both backingstore and screen here?
-        m_backingStore->d->resumeBackingStoreUpdates();
-        m_backingStore->d->resumeScreenUpdates(BackingStore::RenderAndBlit);
-
         // This is where we would restore the browser's chrome
         // if hidden above.
         client()->fullscreenStop();
index a339274..30335b8 100644 (file)
@@ -528,7 +528,7 @@ public:
 #if ENABLE(FULLSCREEN_API)
 #if ENABLE(VIDEO)
     double m_scaleBeforeFullScreen;
-    int m_xScrollOffsetBeforeFullScreen;
+    WebCore::IntPoint m_scrollPositionBeforeFullScreen;
 #endif
 #endif
 
index 07b1619..ec89e2e 100644 (file)
@@ -1,3 +1,32 @@
+2012-12-13  Jacky Jiang  <zhajiang@rim.com>
+
+        [BlackBerry][Fullscreen] Exiting fullscreen does not set the correct scroll position
+        https://bugs.webkit.org/show_bug.cgi?id=104958
+
+        Reviewed by Antonio Gomes.
+        Internally reviewed by Arvid Nilsson and Max Feil.
+
+        PR: 231174
+        When leaving fullscreen, WebPage scale and scroll position can't return
+        to the original scale and scroll position.
+        We can't restore them in WebPagePrivate::exitFullScreenForElement()
+        as they can still be changed thereafter during the async
+        setViewportSize(). And also the async setViewportSize() from the app
+        side isn't guaranteed as some apps don't need to resize the viewport if
+        their windows are already fullscreen.
+        The restoration is basically only needed if viewport size is changed.
+        At the point of entering fullscreen, we can safely assume that there
+        would be a viewport size change thereafter if the current visible size
+        and screen size are not equal. Based on this assumption, we can save
+        the scale and position before entering fullscreen and restore them in
+        setViewportSize() thereafter.
+
+        * Api/WebPage.cpp:
+        (BlackBerry::WebKit::WebPagePrivate::WebPagePrivate):
+        (BlackBerry::WebKit::WebPagePrivate::setViewportSize):
+        * Api/WebPage_p.h:
+        (WebPagePrivate):
+
 2012-12-13  Yong Li  <yoli@rim.com>
 
         [BlackBerry] Possible JS re-entrancy caused by UI event handler