Cherry-pick 272846@main (8395281). https://bugs.webkit.org/show_bug.c…

…gi?id=267331 [iFC][Ruby] frameset with display:none crashes in ruby https://bugs.webkit.org/show_bug.cgi?id=267331 rdar://120496400 Reviewed by Alan Baradlay. <frameset> generates a renderer even with 'display:none' breaking some assumptions. * LayoutTests/fast/ruby/ruby-frameset-display-none-crash-expected.txt: Added. * LayoutTests/fast/ruby/ruby-frameset-display-none-crash.html: Added. * Source/WebCore/html/HTMLFrameElement.cpp: (WebCore::HTMLFrameElement::rendererIsNeeded): * Source/WebCore/html/HTMLFrameSetElement.cpp: (WebCore::HTMLFrameSetElement::rendererIsNeeded): Deleted. * Source/WebCore/html/HTMLFrameSetElement.h: * Source/WebCore/style/StyleAdjuster.cpp: (WebCore::Style::Adjuster::adjust const): Adjust frameset/frame always have 'display:block', even when it is orginally 'none'. This matches other browsers. Canonical link: https://commits.webkit.org/272846@main
WebKit · Jan 28, 2024 · a23f513 · a23f513
1 parent 69ad89f
commit a23f513
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 17 deletions.
diff --git a/LayoutTests/fast/ruby/ruby-frameset-display-none-crash-expected.txt b/LayoutTests/fast/ruby/ruby-frameset-display-none-crash-expected.txt
@@ -0,0 +1 @@
+This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/ruby-frameset-display-none-crash.html b/LayoutTests/fast/ruby/ruby-frameset-display-none-crash.html
@@ -0,0 +1,14 @@
+This test passes if it doesn't crash.
+<style>
+frameset { display:none }
+</style>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function test() {
+    ruby.appendChild(document.createElement("frameset"));
+}
+</script>
+<body onload=test()>
+<ruby id="ruby"></ruby>
diff --git a/Source/WebCore/html/HTMLFrameElement.cpp b/Source/WebCore/html/HTMLFrameElement.cpp
@@ -49,10 +49,9 @@ Ref<HTMLFrameElement> HTMLFrameElement::create(const QualifiedName& tagName, Doc
     return adoptRef(*new HTMLFrameElement(tagName, document));
 }
 
-bool HTMLFrameElement::rendererIsNeeded(const RenderStyle&)
+bool HTMLFrameElement::rendererIsNeeded(const RenderStyle& style)
 {
-    // For compatibility, frames render even when display: none is set.
-    return canLoad();
+    return HTMLFrameElementBase::rendererIsNeeded(style) && canLoad();
 }
 
 RenderPtr<RenderElement> HTMLFrameElement::createElementRenderer(RenderStyle&& style, const RenderTreePosition&)

diff --git a/Source/WebCore/html/HTMLFrameSetElement.cpp b/Source/WebCore/html/HTMLFrameSetElement.cpp
@@ -146,12 +146,6 @@ void HTMLFrameSetElement::attributeChanged(const QualifiedName& name, const Atom
     }
 }
 
-bool HTMLFrameSetElement::rendererIsNeeded(const RenderStyle&)
-{
-    // For compatibility, frames render even when display: none is set.
-    return true;
-}
-
 RenderPtr<RenderElement> HTMLFrameSetElement::createElementRenderer(RenderStyle&& style, const RenderTreePosition&)
 {
     if (style.hasContent())

diff --git a/Source/WebCore/html/HTMLFrameSetElement.h b/Source/WebCore/html/HTMLFrameSetElement.h
@@ -62,7 +62,6 @@ class HTMLFrameSetElement final : public HTMLElement {
     void collectPresentationalHintsForAttribute(const QualifiedName&, const AtomString&, MutableStyleProperties&) final;
 
     void willAttachRenderers() final;
-    bool rendererIsNeeded(const RenderStyle&) final;
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
 
     void defaultEventHandler(Event&) final;

diff --git a/Source/WebCore/style/StyleAdjuster.cpp b/Source/WebCore/style/StyleAdjuster.cpp
@@ -305,19 +305,18 @@ void Adjuster::adjust(RenderStyle& style, const RenderStyle* userAgentAppearance
     if (style.display() == DisplayType::Contents)
         adjustDisplayContentsStyle(style);
 
+    if (m_element && (m_element->hasTagName(frameTag) || m_element->hasTagName(framesetTag))) {
+        // Framesets ignore display and position properties.
+        style.setPosition(PositionType::Static);
+        style.setEffectiveDisplay(DisplayType::Block);
+    }
+
     if (style.display() != DisplayType::None && style.display() != DisplayType::Contents) {
         if (m_element) {
             // Tables never support the -webkit-* values for text-align and will reset back to the default.
             if (is<HTMLTableElement>(*m_element) && (style.textAlign() == TextAlignMode::WebKitLeft || style.textAlign() == TextAlignMode::WebKitCenter || style.textAlign() == TextAlignMode::WebKitRight))
                 style.setTextAlign(TextAlignMode::Start);
 
-            // Frames and framesets never honor position:relative or position:absolute. This is necessary to
-            // fix a crash where a site tries to position these objects. They also never honor display.
-            if (m_element->hasTagName(frameTag) || m_element->hasTagName(framesetTag)) {
-                style.setPosition(PositionType::Static);
-                style.setEffectiveDisplay(DisplayType::Block);
-            }
-
             // Ruby text does not support float or position. This might change with evolution of the specification.
             if (m_element->hasTagName(rtTag)) {
                 style.setPosition(PositionType::Static);