Changeset 126043 in webkit

Timestamp:

Aug 20, 2012 11:11:22 AM (12 years ago)

Author:

bbudge@chromium.org

Message:

webkitfullscreenchange not fired properly in iframe.
​https://bugs.webkit.org/show_bug.cgi?id=93525

Reviewed by Adam Barth.

webkitCancelFullScreen exits fullscreen by invoking webkitExitFullScreen on topDocument.
However, if webkitDidExitFullScreenForElement is invoked on a descendant document, no events
get dispatched. This change starts the event dispatch delay timer on the document where
webkitCancelFullScreen was called, so that the events get dispatched. In addition, when events
are dispatched, the check whether a node has been removed is changed to also check that the
node isn't in another document, as can happen with frames. Finally, webkitExitFullscreen
is fixed to remove unnecessary code and conform to the spec.

No new tests (the existing fullscreen/exit-full-screen-iframe.html test now passes).

• dom/Document.cpp:

(WebCore::Document::webkitExitFullscreen):
(WebCore::Document::webkitDidExitFullScreenForElement):
(WebCore::Document::fullScreenChangeDelayTimerFired):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (5 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-08-20  Bill Budge  <bbudge@chromium.org>
+
+        webkitfullscreenchange not fired properly in iframe.
+        https://bugs.webkit.org/show_bug.cgi?id=93525
+
+        Reviewed by Adam Barth.
+
+        webkitCancelFullScreen exits fullscreen by invoking webkitExitFullScreen on topDocument.
+        However, if webkitDidExitFullScreenForElement is invoked on a descendant document, no events
+        get dispatched. This change starts the event dispatch delay timer on the document where
+        webkitCancelFullScreen was called, so that the events get dispatched. In addition, when events
+        are dispatched, the check whether a node has been removed is changed to also check that the
+        node isn't in another document, as can happen with frames. Finally, webkitExitFullscreen
+        is fixed to remove unnecessary code and conform to the spec.
+
+        No new tests (the existing fullscreen/exit-full-screen-iframe.html test now passes).
+
+        * dom/Document.cpp:
+        (WebCore::Document::webkitExitFullscreen):
+        (WebCore::Document::webkitDidExitFullScreenForElement):
+        (WebCore::Document::fullScreenChangeDelayTimerFired):
+
 2012-08-20  Yuzhu Shen  <yzshen@chromium.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -6 +6 @@
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
- * Copyright (C) 2008, 2009, 2011 Google Inc. All rights reserved.
+ * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) Research In Motion Limited 2010-2011. All rights reserved.
@@ -5555 +5555 @@
         // 2. Queue a task to fire an event named fullscreenchange with its bubbles attribute set to true
         // on doc.
-        Node* target = currentDoc->m_fullScreenElement.get();
-        if (!target)
-            target = currentDoc;
         addDocumentToFullScreenChangeEventQueue(currentDoc);
 
         // 3. If doc's fullscreen element stack is empty and doc's browsing context has a browsing context
         // container, set doc to that browsing context container's node document.
-        if (!newTop && currentDoc->ownerElement())
+        if (!newTop && currentDoc->ownerElement()) {
             currentDoc = currentDoc->ownerElement()->document();
+            continue;
+        }
 
         // 4. Otherwise, set doc to null.
@@ -5676 +5675 @@
     scheduleForcedStyleRecalc();
     
-    m_fullScreenChangeDelayTimer.startOneShot(0);
+    // When webkitCancelFullScreen is called, we call webkitExitFullScreen on the topDocument(). That
+    // means that the events will be queued there. So if we have no events here, start the timer on
+    // the exiting document.
+    Document* exitingDocument = this;
+    if (m_fullScreenChangeEventTargetQueue.isEmpty() && m_fullScreenErrorEventTargetQueue.isEmpty())
+        exitingDocument = topDocument();
+    exitingDocument->m_fullScreenChangeDelayTimer.startOneShot(0);
 }
     
@@ -5747 +5752 @@
             node = documentElement();
 
-        // If the element was removed from our tree, also message the documentElement.
-        if (!contains(node.get()))
+        // If the element was removed from our tree, also message the documentElement. Since we may
+        // have a document hierarchy, check that node isn't in another document.
+        if (!contains(node.get()) && !node->inDocument())
             changeQueue.append(documentElement());
         
@@ -5762 +5768 @@
             node = documentElement();
         
-        // If the node was removed from our tree, also message the documentElement.
-        if (!contains(node.get()))
+        // If the element was removed from our tree, also message the documentElement. Since we may
+        // have a document hierarchy, check that node isn't in another document.
+        if (!contains(node.get()) && !node->inDocument())
             errorQueue.append(documentElement());