Skip to content

Commit acbb891

Browse files
nipunshukla21nipunshukla21
committed
REGRESSION (Safari 26.2): shape-outside initially incorrect when used with imported font
https://bugs.webkit.org/show_bug.cgi?id=304019 rdar://166336491 Reviewed by Simon Fraser and Alan Baradlay. When web fonts load after initial layout, CSS shape properties remain unchanged but box dimensions can change due to different font metrics. The regression caused an early return when CSS is identical, preventing shape invalidation. This issue was fixed by extending the early return condition to check whether Style::Difference requires a repaint or layout. This ensures shapes are invalidated when dimension-affecting properties change. * LayoutTests/fast/shapes/shape-outside-floats/shape-outside-invalidation-after-font-load-expected.txt: Added. * LayoutTests/fast/shapes/shape-outside-floats/shape-outside-invalidation-after-font-load.html: Added. * Source/WebCore/rendering/RenderBox.cpp: (WebCore::RenderBox::styleDidChange): (WebCore::RenderBox::updateShapeOutsideInfoAfterStyleChange): * Source/WebCore/rendering/RenderBox.h: Canonical link: https://commits.webkit.org/305299@main
1 parent 0e7de9c commit acbb891

File tree

4 files changed

+91
-8
lines changed

4 files changed

+91
-8
lines changed

‎LayoutTests/fast/shapes/shape-outside-floats/shape-outside-invalidation-after-font-load-expected.txt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
OOnce upon a time there was text that wrapped around a float.
2+
Test that shape-outside is invalidated when font changes, even when CSS shape properties remain unchanged.
3+
4+
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
5+
6+
7+
PASS Content position changed, indicating shape was properly invalidated
8+
PASS successfullyParsed is true
9+
10+
TEST COMPLETE
11+

‎LayoutTests/fast/shapes/shape-outside-floats/shape-outside-invalidation-after-font-load.html‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<script src="../../../resources/js-test.js"></script>
5+
<style>
6+
body {
7+
margin: 0;
8+
padding: 0;
9+
}
10+
11+
.container {
12+
width: 200px;
13+
font: 20px/1 Ahem;
14+
}
15+
16+
.float-shape {
17+
float: left;
18+
font-size: 60px;
19+
line-height: 1;
20+
font-family: serif;
21+
shape-outside: margin-box;
22+
margin-right: 10px;
23+
background-color: blue;
24+
color: white;
25+
}
26+
</style>
27+
</head>
28+
<body>
29+
<div class="container">
30+
<div class="float-shape" id="float">O</div>
31+
<span id="firstWord">Once</span> upon a time there was text that wrapped around a float.
32+
</div>
33+
<div id="console"></div>
34+
<script>
35+
jsTestIsAsync = true;
36+
37+
function runTest() {
38+
description("Test that shape-outside is invalidated when font changes, even when CSS shape properties remain unchanged.");
39+
40+
const floatElem = document.getElementById('float');
41+
const firstWord = document.getElementById('firstWord');
42+
43+
document.body.offsetTop;
44+
45+
const initialLeft = firstWord.getBoundingClientRect().left;
46+
const initialTop = firstWord.getBoundingClientRect().top;
47+
48+
floatElem.style.fontFamily = 'Ahem';
49+
50+
document.body.offsetTop;
51+
52+
const newLeft = firstWord.getBoundingClientRect().left;
53+
const newTop = firstWord.getBoundingClientRect().top;
54+
55+
const positionChanged = (Math.abs(newLeft - initialLeft) > 1) || (Math.abs(newTop - initialTop) > 1);
56+
57+
if (positionChanged) {
58+
testPassed("Content position changed, indicating shape was properly invalidated");
59+
} else {
60+
testFailed("Content position unchanged. Shape was not invalidated when font metrics changed.");
61+
}
62+
63+
finishJSTest();
64+
}
65+
66+
if (window.testRunner) {
67+
testRunner.dumpAsText();
68+
testRunner.waitUntilDone();
69+
}
70+
71+
window.addEventListener('load', () => {
72+
runTest();
73+
});
74+
</script>
75+
</body>
76+
</html>

‎Source/WebCore/rendering/RenderBox.cpp‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ void RenderBox::styleDidChange(Style::Difference diff, const RenderStyle* oldSty
415415
}
416416

417417
if ((oldStyle && !oldStyle->shapeOutside().isNone()) || !style().shapeOutside().isNone())
418-
updateShapeOutsideInfoAfterStyleChange(style(), oldStyle);
418+
updateShapeOutsideInfoAfterStyleChange(style(), oldStyle, diff);
419419
updateGridPositionAfterStyleChange(style(), oldStyle);
420420

421421
// Changing the position from/to absolute can potentially create/remove flex/grid items, as absolutely positioned
@@ -474,19 +474,15 @@ void RenderBox::updateGridPositionAfterStyleChange(const RenderStyle& style, con
474474
parentGrid->setNeedsItemPlacement();
475475
}
476476

477-
void RenderBox::updateShapeOutsideInfoAfterStyleChange(const RenderStyle& style, const RenderStyle* oldStyle)
477+
void RenderBox::updateShapeOutsideInfoAfterStyleChange(const RenderStyle& style, const RenderStyle* oldStyle, Style::Difference diff)
478478
{
479479
Style::ShapeOutside shapeOutside = style.shapeOutside();
480480
Style::ShapeOutside oldShapeOutside = oldStyle ? oldStyle->shapeOutside() : Style::ComputedStyle::initialShapeOutside();
481481

482482
Style::ShapeMargin shapeMargin = style.shapeMargin();
483483
Style::ShapeMargin oldShapeMargin = oldStyle ? oldStyle->shapeMargin() : Style::ComputedStyle::initialShapeMargin();
484484

485-
Style::ShapeImageThreshold shapeImageThreshold = style.shapeImageThreshold();
486-
Style::ShapeImageThreshold oldShapeImageThreshold = oldStyle ? oldStyle->shapeImageThreshold() : Style::ComputedStyle::initialShapeImageThreshold();
487-
488-
// FIXME: A future optimization would do a deep comparison for equality. (bug 100811)
489-
if (shapeOutside == oldShapeOutside && shapeMargin == oldShapeMargin && shapeImageThreshold == oldShapeImageThreshold)
485+
if (diff <= Style::DifferenceResult::RecompositeLayer)
490486
return;
491487

492488
if (shapeOutside.isNone())

‎Source/WebCore/rendering/RenderBox.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ class RenderBox : public RenderBoxModelObject {
698698
private:
699699
void addOverflowWithRendererOffset(const RenderBox&, LayoutSize, OptionSet<ComputeOverflowOptions> = { });
700700

701-
void updateShapeOutsideInfoAfterStyleChange(const RenderStyle&, const RenderStyle* oldStyle);
701+
void updateShapeOutsideInfoAfterStyleChange(const RenderStyle&, const RenderStyle* oldStyle, Style::Difference);
702702

703703
void updateGridPositionAfterStyleChange(const RenderStyle&, const RenderStyle* oldStyle);
704704

0 commit comments

Comments
 (0)