Title: [136331] trunk
- Revision
- 136331
- Author
- [email protected]
- Date
- 2012-12-02 03:38:01 -0800 (Sun, 02 Dec 2012)
Log Message
documentElement should not always get a renderer
https://bugs.webkit.org/show_bug.cgi?id=103475
Patch by Elliott Sprehn <[email protected]> on 2012-12-02
Reviewed by Ojan Vafai.
Source/WebCore:
Only SVG documents need a renderer on the documentElement when it's
display: none. This behavior is wrong, but there's assumptions in
FrameView and SVGImage that depend on a RenderSVGRoot existing for all
loaded SVG document and images.
The current code has logic that makes it appear as if we always create
a renderer for the documentElement even when it's display: none, but
because HTMLElement::createRenderer doesn't call super we end up with
HTMLElement::rendererIsNeeded returning true for display: none, but
HTMLElement::createRenderer returning null.
Instead of this generic documentElement logic only special case SVG
until someone can fix Bug 103493 so we don't need it.
Test: fast/css/display-none-documentElement.html
* dom/Element.cpp:
(WebCore::Element::rendererIsNeeded):
(WebCore::Element::createRenderer):
* svg/SVGSVGElement.cpp:
(WebCore::SVGSVGElement::rendererIsNeeded):
* svg/SVGSVGElement.h:
(SVGSVGElement):
LayoutTests:
Test that using display: none on the documentElement hides the document.
* fast/css/display-none-documentElement-expected.html: Added.
* fast/css/display-none-documentElement.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (136330 => 136331)
--- trunk/LayoutTests/ChangeLog 2012-12-02 11:30:50 UTC (rev 136330)
+++ trunk/LayoutTests/ChangeLog 2012-12-02 11:38:01 UTC (rev 136331)
@@ -1,3 +1,15 @@
+2012-12-02 Elliott Sprehn <[email protected]>
+
+ documentElement should not always get a renderer
+ https://bugs.webkit.org/show_bug.cgi?id=103475
+
+ Reviewed by Ojan Vafai.
+
+ Test that using display: none on the documentElement hides the document.
+
+ * fast/css/display-none-documentElement-expected.html: Added.
+ * fast/css/display-none-documentElement.html: Added.
+
2012-12-02 Pan Deng <[email protected]>
[Resource Timing]Test cases of cross origin resource timing.
Added: trunk/LayoutTests/fast/css/display-none-documentElement-expected.html (0 => 136331)
--- trunk/LayoutTests/fast/css/display-none-documentElement-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css/display-none-documentElement-expected.html 2012-12-02 11:38:01 UTC (rev 136331)
@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+
+<iframe></iframe>
+
+<iframe></iframe>
Added: trunk/LayoutTests/fast/css/display-none-documentElement.html (0 => 136331)
--- trunk/LayoutTests/fast/css/display-none-documentElement.html (rev 0)
+++ trunk/LayoutTests/fast/css/display-none-documentElement.html 2012-12-02 11:38:01 UTC (rev 136331)
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+
+<iframe srcdoc="<html style='display: none'>Foo"></iframe>
+
+<iframe id="doc"></iframe>
+
+<script>
+ // Test display:none on a non-HTMLElement that is the documentElement.
+ var doc = document.getElementById('doc').contentDocument;
+ var root = doc.implementation.createDocument().createElement('not-a-real-element');
+ doc.replaceChild(root, doc.documentElement);
+ root.appendChild(document.createElement('style')).textContent = 'not-a-real-element { display: none }';
+ root.appendChild(document.createElement('p')).textContent = 'Should not be visible';
+</script>
Modified: trunk/Source/WebCore/ChangeLog (136330 => 136331)
--- trunk/Source/WebCore/ChangeLog 2012-12-02 11:30:50 UTC (rev 136330)
+++ trunk/Source/WebCore/ChangeLog 2012-12-02 11:38:01 UTC (rev 136331)
@@ -1,3 +1,34 @@
+2012-12-02 Elliott Sprehn <[email protected]>
+
+ documentElement should not always get a renderer
+ https://bugs.webkit.org/show_bug.cgi?id=103475
+
+ Reviewed by Ojan Vafai.
+
+ Only SVG documents need a renderer on the documentElement when it's
+ display: none. This behavior is wrong, but there's assumptions in
+ FrameView and SVGImage that depend on a RenderSVGRoot existing for all
+ loaded SVG document and images.
+
+ The current code has logic that makes it appear as if we always create
+ a renderer for the documentElement even when it's display: none, but
+ because HTMLElement::createRenderer doesn't call super we end up with
+ HTMLElement::rendererIsNeeded returning true for display: none, but
+ HTMLElement::createRenderer returning null.
+
+ Instead of this generic documentElement logic only special case SVG
+ until someone can fix Bug 103493 so we don't need it.
+
+ Test: fast/css/display-none-documentElement.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::rendererIsNeeded):
+ (WebCore::Element::createRenderer):
+ * svg/SVGSVGElement.cpp:
+ (WebCore::SVGSVGElement::rendererIsNeeded):
+ * svg/SVGSVGElement.h:
+ (SVGSVGElement):
+
2012-12-02 Pan Deng <[email protected]>
[Resource Timing] implementation of cross origin resouce timing restrictions.
Modified: trunk/Source/WebCore/dom/Element.cpp (136330 => 136331)
--- trunk/Source/WebCore/dom/Element.cpp 2012-12-02 11:30:50 UTC (rev 136330)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-12-02 11:38:01 UTC (rev 136331)
@@ -1054,18 +1054,11 @@
bool Element::rendererIsNeeded(const NodeRenderingContext& context)
{
- return (document()->documentElement() == this) || (context.style()->display() != NONE);
+ return context.style()->display() != NONE;
}
-RenderObject* Element::createRenderer(RenderArena* arena, RenderStyle* style)
+RenderObject* Element::createRenderer(RenderArena*, RenderStyle* style)
{
- if (document()->documentElement() == this && style->display() == NONE) {
- // Ignore display: none on root elements. Force a display of block in that case.
- RenderBlock* result = new (arena) RenderBlock(this);
- if (result)
- result->setAnimatableStyle(style);
- return result;
- }
return RenderObject::createObject(this, style);
}
Modified: trunk/Source/WebCore/svg/SVGSVGElement.cpp (136330 => 136331)
--- trunk/Source/WebCore/svg/SVGSVGElement.cpp 2012-12-02 11:30:50 UTC (rev 136330)
+++ trunk/Source/WebCore/svg/SVGSVGElement.cpp 2012-12-02 11:38:01 UTC (rev 136331)
@@ -470,6 +470,17 @@
return transform.multiply(viewBoxTransform);
}
+bool SVGSVGElement::rendererIsNeeded(const NodeRenderingContext& context)
+{
+ // FIXME: We should respect display: none on the documentElement svg element
+ // but many things in FrameView and SVGImage depend on the RenderSVGRoot when
+ // they should instead depend on the RenderView.
+ // https://bugs.webkit.org/show_bug.cgi?id=103493
+ if (document()->documentElement() == this)
+ return true;
+ return StyledElement::rendererIsNeeded(context);
+}
+
RenderObject* SVGSVGElement::createRenderer(RenderArena* arena, RenderStyle*)
{
if (isOutermostSVGSVGElement())
Modified: trunk/Source/WebCore/svg/SVGSVGElement.h (136330 => 136331)
--- trunk/Source/WebCore/svg/SVGSVGElement.h 2012-12-02 11:30:50 UTC (rev 136330)
+++ trunk/Source/WebCore/svg/SVGSVGElement.h 2012-12-02 11:38:01 UTC (rev 136331)
@@ -146,7 +146,7 @@
virtual void parseAttribute(const QualifiedName&, const AtomicString&) OVERRIDE;
- virtual bool rendererIsNeeded(const NodeRenderingContext& context) { return StyledElement::rendererIsNeeded(context); }
+ virtual bool rendererIsNeeded(const NodeRenderingContext&) OVERRIDE;
virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
virtual InsertionNotificationRequest insertedInto(ContainerNode*) OVERRIDE;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes