Attachment 346875 Details for Bug 187990 – Patch and layout tests

[patch] Patch and layout tests

bug-187990-20180809164813.patch (text/plain), 9.60 KB, created by Daniel Bates on 2018-08-09 16:48:14 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2018-08-09 16:48:14 PDT

Size: 9.60 KB

patch

obsolete

>Subversion Revision: 234703
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c79ea1784334bde42d555278c04216f9a960db53..ebabcc3de7e0c3061fbdacfb77ee2dc3eabbfb74 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+2018-08-09  Daniel Bates  <dabates@apple.com>
>+
>+        Once <object> is hidden, its content won't be displayed again if its URL has fragment identifier ("#").
>+        https://bugs.webkit.org/show_bug.cgi?id=187990
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fixes and issue where an HTML object element that behaves like an iframe and references a resource
>+        whose URL contains a fragment would not be repainted when its CSS display property changes. 
>+
>+        Rendering of an <object> that behaves like an iframe is handled by a widget (FrameView). When
>+        the CSS display property for an <object> is set to "none" we detach the widget from its renderer
>+        as part of destroying the render tree for the <object>. Subsequently changing the CSS display
>+        to a non-"none"/"contents" value will create a new renderer for <object> R. For an <object> that
>+        behaves like an iframe, we navigate to the resource associated with the <object> reusing the
>+        existing Frame F object created when we first displayed the <object>. Unlike the case where
>+        the URL of the resource does not contain a fragment, navigating to a fragment in the same document
>+        uses a different code path that does not re-associate the FrameView of F with R before it scrolls
>+        the view; => the FrameView of F is not in the view hierarchy. Therefore we do not paint anything
>+        for the content of the <object> and hence the scroll is not observable. Instead we need to ensure
>+        that the FrameView of F installed in the view hierarchy when navigating to an anchor in the same
>+        document.
>+
>+        As a side benefit of this fix we now also repaint an <object> that is programmatically navigated
>+        to a different anchor in the same document.
>+
>+        Tests: fast/repaint/object-as-iframe-hide-and-show-document-at-anchor.html
>+               fast/repaint/object-as-iframe-navigate-to-same-document-anchor-repaint.html
>+
>+        * loader/FrameLoader.cpp:
>+        (WebCore::FrameLoader::loadInSameDocument):
>+
> 2018-08-08  Daniel Bates  <dabates@apple.com>
> 
>         Disallow navigations when page cache updates the current document of the frame
>diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
>index 1845613c79c0ee497481db9a1e6e7e29a122744b..14c3fc81c2790db6248fa43930a2f94b474adf3c 100644
>--- a/Source/WebCore/loader/FrameLoader.cpp
>+++ b/Source/WebCore/loader/FrameLoader.cpp
>@@ -1149,6 +1149,13 @@ void FrameLoader::loadInSameDocument(const URL& url, SerializedScriptValue* stat
>     // Otherwise, the parent frame may think we never finished loading.
>     started();
> 
>+    if (auto* ownerElement = m_frame.ownerElement()) {
>+        auto* ownerRenderer = ownerElement->renderer();
>+        auto* view = m_frame.view();
>+        if (is<RenderWidget>(ownerRenderer) && view)
>+            downcast<RenderWidget>(*ownerRenderer).setWidget(view);
>+    }
>+
>     // We need to scroll to the fragment whether or not a hash change occurred, since
>     // the user might have scrolled since the previous navigation.
>     scrollToFragmentWithParentBoundary(url, isNewNavigation);
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 5898c9548976c80ec6753ffa2a3ed1ea11ea7c93..4c091f3612b2cc84bc0600af6fe0e520a87b732a 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-08-09  Daniel Bates  <dabates@apple.com>
>+
>+        Once <object> is hidden, its content won't be displayed again if its URL has fragment identifier ("#").
>+        https://bugs.webkit.org/show_bug.cgi?id=187990
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add tests to ensure that we repaint an <object>, whose resource URL contains a fragment, when
>+        its CSS display property changes as well as when its navigated to a different anchor in the same
>+        document.
>+
>+        * fast/repaint/object-as-iframe-hide-and-show-document-at-anchor-expected.txt: Added.
>+        * fast/repaint/object-as-iframe-hide-and-show-document-at-anchor.html: Added.
>+        * fast/repaint/object-as-iframe-navigate-to-same-document-anchor-repaint-expected.txt: Added.
>+        * fast/repaint/object-as-iframe-navigate-to-same-document-anchor-repaint.html: Added.
>+        * fast/repaint/resources/red-square-on-top-of-green-square.html: Added.
>+
> 2018-08-08  Daniel Bates  <dabates@apple.com>
> 
>         Disallow navigations when page cache updates the current document of the frame
>diff --git a/LayoutTests/fast/repaint/object-as-iframe-hide-and-show-document-at-anchor-expected.txt b/LayoutTests/fast/repaint/object-as-iframe-hide-and-show-document-at-anchor-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..c221fd68cefb0cec5b91f41780da8469cd62e6af
>--- /dev/null
>+++ b/LayoutTests/fast/repaint/object-as-iframe-hide-and-show-document-at-anchor-expected.txt
>@@ -0,0 +1,14 @@
>+layer at (0,0) size 800x600
>+  RenderView at (0,0) size 800x600
>+layer at (0,0) size 800x278
>+  RenderBlock {HTML} at (0,0) size 800x278
>+    RenderBody {BODY} at (8,8) size 784x262
>+      RenderEmbeddedObject {OBJECT} at (0,0) size 258x258 [border: (1px solid #000000)]
>+        layer at (0,0) size 256x512
>+          RenderView at (0,0) size 241x241
>+        layer at (0,0) size 241x512
>+          RenderBlock {HTML} at (0,0) size 241x512
>+            RenderBody {BODY} at (0,0) size 241x512
>+              RenderBlock {DIV} at (0,0) size 256x256 [bgcolor=#FF0000]
>+              RenderBlock {DIV} at (0,256) size 256x256 [bgcolor=#008000]
>+      RenderText {#text} at (0,0) size 0x0
>diff --git a/LayoutTests/fast/repaint/object-as-iframe-hide-and-show-document-at-anchor.html b/LayoutTests/fast/repaint/object-as-iframe-hide-and-show-document-at-anchor.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..181d89f6e60fc6f30aa28c5d5b8e2ac464148ac0
>--- /dev/null
>+++ b/LayoutTests/fast/repaint/object-as-iframe-hide-and-show-document-at-anchor.html
>@@ -0,0 +1,22 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<style>
>+object {
>+	border: 1px solid black;
>+}
>+</style>
>+<script>
>+function runTest()
>+{
>+	let testElement = document.getElementById("test");
>+	testElement.style.display = "none";
>+	testElement.style.display = "inline";
>+}
>+</script>
>+</head>
>+<body>
>+
>+<object id="test" data="resources/red-square-on-top-of-green-square.html#green" width="256" height="256" onload="runTest()"></object>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/repaint/object-as-iframe-navigate-to-same-document-anchor-repaint-expected.txt b/LayoutTests/fast/repaint/object-as-iframe-navigate-to-same-document-anchor-repaint-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..c221fd68cefb0cec5b91f41780da8469cd62e6af
>--- /dev/null
>+++ b/LayoutTests/fast/repaint/object-as-iframe-navigate-to-same-document-anchor-repaint-expected.txt
>@@ -0,0 +1,14 @@
>+layer at (0,0) size 800x600
>+  RenderView at (0,0) size 800x600
>+layer at (0,0) size 800x278
>+  RenderBlock {HTML} at (0,0) size 800x278
>+    RenderBody {BODY} at (8,8) size 784x262
>+      RenderEmbeddedObject {OBJECT} at (0,0) size 258x258 [border: (1px solid #000000)]
>+        layer at (0,0) size 256x512
>+          RenderView at (0,0) size 241x241
>+        layer at (0,0) size 241x512
>+          RenderBlock {HTML} at (0,0) size 241x512
>+            RenderBody {BODY} at (0,0) size 241x512
>+              RenderBlock {DIV} at (0,0) size 256x256 [bgcolor=#FF0000]
>+              RenderBlock {DIV} at (0,256) size 256x256 [bgcolor=#008000]
>+      RenderText {#text} at (0,0) size 0x0
>diff --git a/LayoutTests/fast/repaint/object-as-iframe-navigate-to-same-document-anchor-repaint.html b/LayoutTests/fast/repaint/object-as-iframe-navigate-to-same-document-anchor-repaint.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..d42a83e66625aa38fa5d3166ebfae0d1bf561c0d
>--- /dev/null
>+++ b/LayoutTests/fast/repaint/object-as-iframe-navigate-to-same-document-anchor-repaint.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<style>
>+object {
>+	border: 1px solid black;
>+}
>+</style>
>+<script>
>+function changeURLFragment(objectElement)
>+{
>+    objectElement.data = objectElement.data + "#green";
>+}
>+</script>
>+</head>
>+<body>
>+
>+<object data="resources/red-square-on-top-of-green-square.html" width="256" height="256" onload="changeURLFragment(this)"></object>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/repaint/resources/red-square-on-top-of-green-square.html b/LayoutTests/fast/repaint/resources/red-square-on-top-of-green-square.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..7828a8a89a3a2ebcc12396d5ed970ff1fb494f6f
>--- /dev/null
>+++ b/LayoutTests/fast/repaint/resources/red-square-on-top-of-green-square.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<style>
>+body {
>+	margin: 0;
>+	padding: 0;
>+}
>+
>+.square {
>+	height: 256px;
>+	width: 256px;
>+}
>+</style>
>+</head>
>+<body>
>+<div class="square" style="background-color: red" id="red"></div>
>+<div class="square" style="background-color: green" id="green"></div>
>+</body>
>+</html>

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 187990: 345746 | 346875