Skip to content

Commit 56e2ac8

Browse files
Calme1709kalenikaliaksandr
authored andcommitted
LibWeb: Always do parent document layout updates before updating style
We were doing this manually within `Document::update_layout()` and `CSSStyleProperties::get_direct_property()` but we should do it for all callers of `Document::update_style()`
1 parent b5db79b commit 56e2ac8

File tree

5 files changed

+38
-13
lines changed

5 files changed

+38
-13
lines changed

Libraries/LibWeb/CSS/CSSStyleProperties.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -517,14 +517,6 @@ Optional<StyleProperty> CSSStyleProperties::get_direct_property(PropertyNameAndI
517517

518518
Layout::NodeWithStyle* layout_node = abstract_element.layout_node();
519519

520-
// Pending changes to an ancestor document's layout can affect an element's computed style e.g. an IFrame's
521-
// width being changed can affect media query evaluation and the value of the `vw` unit.
522-
// FIXME: This is likely overkill and can be optimized
523-
for (auto const& navigable : abstract_element.document().ancestor_navigables()) {
524-
if (navigable->active_document())
525-
navigable->active_document()->update_layout(DOM::UpdateLayoutReason::ResolvedCSSStyleDeclarationProperty);
526-
}
527-
528520
// FIXME: Be smarter about updating layout if there's no layout node.
529521
// We may legitimately have no layout node if we're not visible, but this protects against situations
530522
// where we're requesting the computed style before layout has happened.

Libraries/LibWeb/DOM/Document.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,11 +1342,6 @@ void Document::update_layout(UpdateLayoutReason reason)
13421342
if (!navigable || navigable->active_document() != this)
13431343
return;
13441344

1345-
// NOTE: If our parent document needs a relayout, we must do that *first*.
1346-
// This is necessary as the parent layout may cause our viewport to change.
1347-
if (navigable->container() && &navigable->container()->document() != this)
1348-
navigable->container()->document().update_layout(reason);
1349-
13501345
update_style();
13511346

13521347
if (m_layout_root && !m_layout_root->needs_layout_update())
@@ -1557,6 +1552,11 @@ void Document::update_layout(UpdateLayoutReason reason)
15571552

15581553
void Document::update_style()
15591554
{
1555+
// NOTE: If our parent document needs a relayout, we must do that *first*. This is required as it may cause the
1556+
// viewport to change which will can affect media query evaluation and the value of the `vw` unit.
1557+
if (navigable()->container() && &navigable()->container()->document() != this)
1558+
navigable()->container()->document().update_layout(UpdateLayoutReason::ChildDocumentStyleUpdate);
1559+
15601560
if (!browsing_context())
15611561
return;
15621562

Libraries/LibWeb/DOM/Document.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ enum class InvalidateLayoutTreeReason {
7878
X(CanvasRenderingContext2DSetStrokeStyle) \
7979
X(CanvasSetFillStyle) \
8080
X(CursorBlinkTimer) \
81+
X(ChildDocumentStyleUpdate) \
8182
X(Debugging) \
8283
X(DocumentElementFromPoint) \
8384
X(DocumentElementsFromPoint) \
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
1
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<iframe
4+
width="499"
5+
id="frame"
6+
srcdoc="
7+
<!DOCTYPE html>
8+
<html>
9+
<style>
10+
@keyframes foo {}
11+
12+
@media (width >= 500px) {
13+
#bar {
14+
animation: foo 1s;
15+
}
16+
}
17+
</style>
18+
<div id='bar'></div>
19+
</html>
20+
"
21+
></iframe>
22+
<script src="../include.js"></script>
23+
<script>
24+
promiseTest(async () => {
25+
await new Promise(resolve => window.addEventListener("load", resolve));
26+
27+
frame.width = "500";
28+
println(frame.contentDocument.getAnimations().length);
29+
});
30+
</script>
31+
</html>

0 commit comments

Comments
 (0)