Skip to content
Permalink
Browse files
[LBSE] Assure <foreignObject> HTML descendants create a new formattin…
…g context

https://bugs.webkit.org/show_bug.cgi?id=245908

Reviewed by Simon Fraser.

Fix mistake in svg/hixie/mixed/009.xml and its copies. A style sheet
applied a certain 'margin' value to all <div> elements, including the
one inside the <foreignObject>, which was unintentional. Now the testcase
behaves as expected in Firefox/Chrome, but is broken in Safari. With LBSE
the test works correctly: 'margin' is respected as intended on block-level
children that are direct children of <foreignObject>.

-> <foreignObject> needs to create a new formatting context for its descendants.
This finally fixes margin handling for block-children of <foreignObject> which
was broken since forever (at least 15 years) in WebKit.

Some further fixes are necessary to correctly compute clip rects for <foreignObject>.
<fO> should behaves like an absolutely positioned object -- but we failed
to honor that SVG2 requirement, when comuting clip rects in RenderLayer - fix that.

Enable some tests that were skipped because of bugs in the testcases themselves,
such as missing width/height attributes on <foreignObject> elements.

Covered by existing tests, and a specific new test that enforce LBSE usage so we
get coverage for this in EWS layout test runs, where LBSE is not explicitely
turned out (similar to the tests in svg/z-index, svg/compositing).

* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/mac-ventura-wk2-lbse-text/TestExpectations:
* LayoutTests/platform/mac-ventura-wk2-lbse-text/svg/hixie/mixed/009-expected.txt:
* LayoutTests/platform/win/TestExpectations:
* LayoutTests/svg/custom/display-table-caption-foreignObject.svg:
* LayoutTests/svg/custom/display-table-caption-inherit-foreignObject.xhtml:
* LayoutTests/svg/custom/use-on-use-with-child.svg:
* LayoutTests/svg/dom/SVGScriptElement/script-async-attr.svg:
* LayoutTests/svg/dom/SVGScriptElement/script-load-and-error-events.svg:
* LayoutTests/svg/dom/SVGScriptElement/script-onerror-bubbling.svg:
* LayoutTests/svg/dom/SVGScriptElement/script-reexecution.svg:
* LayoutTests/svg/dom/SVGScriptElement/script-type-attribute.svg:
* LayoutTests/svg/dom/smil-methods.svg:
* LayoutTests/svg/foreignObject/respect-block-margin-expected.html: Added.
* LayoutTests/svg/foreignObject/respect-block-margin.html: Added.
* LayoutTests/svg/hittest/svg-standalone-tooltip.svg:
* LayoutTests/svg/hixie/mixed/009.xml:
* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::createsNewFormattingContext const):
* Source/WebCore/rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateAncestorDependentState):
(WebCore::RenderLayer::calculateClipRects const):
* Source/WebCore/rendering/RenderObject.h:
(WebCore::RenderObject::shouldUsePositionedClipping const):

Canonical link: https://commits.webkit.org/256960@main
  • Loading branch information
nikolaszimmermann committed Nov 23, 2022
1 parent 8b10813 commit 7fd975ae74272051463b1f88a67cffdf7c5dfcc4
Show file tree
Hide file tree
Showing 20 changed files with 54 additions and 33 deletions.
@@ -1305,9 +1305,10 @@ webkit.org/b/240934 imported/w3c/web-platform-tests/css/filter-effects/root-elem

imported/w3c/web-platform-tests/css/filter-effects/css-filters-animation-opacity.html [ ImageOnlyFailure ]

# Compositing/z-index needs LBSE enabled builds, which is off by default for this port.
# The following tests need LBSE enabled builds, which is off by default for this port.
svg/compositing [ Skip ]
svg/z-index [ Skip ]
svg/foreignObject/respect-block-margin.html [ Skip ]

# SVG tests that are broken in the legacy SVG engine, but pass using LBSE.
svg/custom/circle-move-invalidation-small-viewBox.svg [ ImageOnlyFailure ]
@@ -571,7 +571,6 @@ svg/custom/multiple-view-elements.html [ ImageOnlyFailure ]
svg/dom/complex-svgView-specification.html [ ImageOnlyFailure ]

# SVG <foreignObject> issues
svg/hixie/mixed/009.xml [ ImageOnlyFailure ]
svg/overflow/overflow-on-foreignObject.svg [ ImageOnlyFailure ]

# SVG <image> (buffered-rendering) issues
@@ -580,19 +579,8 @@ svg/repaint/buffered-rendering-static-image.html [ ImageOnlyFailure ]
###############
# Buggy tests #
###############
# 1) no width/height specified for <foreignObject> (disabled rendering in LBSE, ignored by legacy)
svg/custom/display-table-caption-foreignObject.svg [ Failure ]
svg/custom/display-table-caption-inherit-foreignObject.xhtml [ Failure ]
svg/custom/use-on-use-with-child.svg [ Failure ]
svg/dom/SVGScriptElement/script-async-attr.svg [ Failure ]
svg/dom/SVGScriptElement/script-load-and-error-events.svg [ Failure ]
svg/dom/SVGScriptElement/script-onerror-bubbling.svg [ Failure ]
svg/dom/SVGScriptElement/script-reexecution.svg [ Failure ]
svg/dom/SVGScriptElement/script-type-attribute.svg [ Failure ]
svg/dom/smil-methods.svg [ Failure ]
svg/hittest/svg-standalone-tooltip.svg [ Failure ]

# 2) Parses render tree output, therefore not adapted to LBSE.

# Parses render tree output, therefore not adapted to LBSE.
svg/outermost-svg-root.html [ Failure ]

###############
@@ -17,8 +17,8 @@ layer at (8,38) size 400x120
RenderSVGViewportContainer at (0,0) size 400x120
layer at (8,38) size 60x12
RenderSVGRect {rect} at (0,0) size 60x12 [fill={[type=SOLID] [color=#EEEEEE]}] [x=0.00] [y=0.00] [width=60.00] [height=12.00]
layer at (8,38) size 60x10 scrollHeight 33
layer at (8,38) size 60x10 scrollHeight 13
RenderSVGForeignObject {foreignObject} at (0,0) size 60x10
RenderBlock {div} at (0,10) size 60x13 [color=#000080]
RenderBlock {div} at (0,0) size 60x13 [color=#000080]
RenderText {#text} at (0,0) size 24x13
text run at (0,0) width 24: "TEST"
@@ -2259,9 +2259,10 @@ svg/filters/feDisplacementMap-filterUnits.svg [ ImageOnlyFailure ]
# GCController.collect() issues.
svg/animations/svglength-element-removed-crash.svg [ Failure ]

# Compositing/z-index needs LBSE enabled builds, which is off by default for this port.
# The following tests need LBSE enabled builds, which is off by default for this port.
svg/compositing [ Skip ]
svg/z-index [ Skip ]
svg/foreignObject/respect-block-margin.html [ Skip ]

# SVG tests that are broken in the legacy SVG engine, but pass using LBSE.
svg/custom/circle-move-invalidation-small-viewBox.svg [ ImageOnlyFailure ]
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -4,6 +4,6 @@
if (window.testRunner)
testRunner.dumpAsText();
</script>
<foreignObject display="inherit"><xhtml:div>This test PASSED if we don't crash when the display value is table-caption by using inherit</xhtml:div></foreignObject>
<foreignObject width="1" height="1" display="inherit"><xhtml:div>This test PASSED if we don't crash when the display value is table-caption by using inherit</xhtml:div></foreignObject>
</svg>
</div>
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<html>
<head>
<body style="margin: 0">
<div style="width: 400px; height: 120px;">
<div style="margin: 100px 0">Margin should be respected</div>
</div>
</body>
</html>
@@ -0,0 +1,11 @@
<!DOCTYPE html><!-- webkit-test-runner [ LayerBasedSVGEngineEnabled=true ] -->
<html>
<head>
<body style="margin: 0">
<svg xmlns="http://www.w3.org/2000/svg" width="400" height="120">
<foreignObject width="400" height="120" overflow="visible">
<div style="margin: 100px 0">Margin should be respected</div>
</foreignObject>
</svg>
</body>
</html>
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -12,9 +12,9 @@
<svg xmlns="http://www.w3.org/2000/svg" width="400" height="120" class="test">
<rect x="0" y="0" width="60" height="12" transform="scale(10)"/>
<foreignObject x="0" y="0" width="60" height="10" transform="scale(10)">
<div xmlns="http://www.w3.org/1999/xhtml"> TEST </div>
<div xmlns="http://www.w3.org/1999/xhtml" style="margin: 0;"> TEST </div>
</foreignObject>
</svg>
<div class="control">TEST</div>
</body>
</html>
</html>
@@ -2134,7 +2134,7 @@ bool RenderElement::createsNewFormattingContext() const
if (isWritingModeRoot() && isBlockContainer())
return true;
return isInlineBlockOrInlineTable() || isFlexItemIncludingDeprecated()
|| isTableCell() || isTableCaption() || isFieldset() || isDocumentElementRenderer() || isRenderFragmentedFlow()
|| isTableCell() || isTableCaption() || isFieldset() || isDocumentElementRenderer() || isRenderFragmentedFlow() || isSVGForeignObject()
|| style().specifiesColumns() || style().columnSpan() == ColumnSpan::All || style().display() == DisplayType::FlowRoot || establishesIndependentFormattingContext();
}

@@ -105,6 +105,7 @@
#include "RenderMarquee.h"
#include "RenderMultiColumnFlow.h"
#include "RenderReplica.h"
#include "RenderSVGForeignObject.h"
#include "RenderSVGHiddenContainer.h"
#include "RenderSVGInline.h"
#include "RenderSVGModelObject.h"
@@ -1560,7 +1561,16 @@ void RenderLayer::setAncestorChainHasVisibleDescendant()

void RenderLayer::updateAncestorDependentState()
{
bool insideSVGForeignObject = renderer().document().mayHaveRenderedSVGForeignObjects() && ancestorsOfType<LegacyRenderSVGForeignObject>(renderer()).first();
bool insideSVGForeignObject = false;
if (renderer().document().mayHaveRenderedSVGForeignObjects()) {
if (ancestorsOfType<LegacyRenderSVGForeignObject>(renderer()).first())
insideSVGForeignObject = true;
#if ENABLE(LAYER_BASED_SVG_ENGINE)
else if (renderer().document().settings().layerBasedSVGEngineEnabled() && ancestorsOfType<RenderSVGForeignObject>(renderer()).first())
insideSVGForeignObject = true;
#endif
}

if (insideSVGForeignObject == m_insideSVGForeignObject)
return;

@@ -4407,7 +4417,7 @@ void RenderLayer::calculateClipRects(const ClipRectsContext& clipRectsContext, C
clipRects.setFixed(true);
} else if (renderer().isInFlowPositioned())
clipRects.setPosClipRect(clipRects.overflowClipRect());
else if (renderer().isAbsolutelyPositioned())
else if (renderer().shouldUsePositionedClipping())
clipRects.setOverflowClipRect(clipRects.posClipRect());

// Update the clip rects that will be passed to child layers.
@@ -428,6 +428,7 @@ class RenderObject : public CachedImageClient {
bool isAbsolutelyPositioned() const { return isOutOfFlowPositioned() && style().position() == PositionType::Absolute; }
bool isRelativelyPositioned() const { return m_bitfields.isRelativelyPositioned(); }
bool isStickilyPositioned() const { return m_bitfields.isStickilyPositioned(); }
bool shouldUsePositionedClipping() const { return isAbsolutelyPositioned() || isSVGForeignObject(); }

bool isText() const { return !m_bitfields.isBox() && m_bitfields.isTextOrRenderView(); }
bool isLineBreak() const { return m_bitfields.isLineBreak(); }

0 comments on commit 7fd975a

Please sign in to comment.