Skip to content

Commit

Permalink
canvas-as-container-005.html & canvas-as-container-006.html fail
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=253936
rdar://106739131

Reviewed by Alan Baradlay.

When resolving computed style in a non-rendered subtree we fail to take container queries into account.

* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/canvas-as-container-005-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/canvas-as-container-006-expected.txt:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::styleForElementIgnoringPendingStylesheets):

Take care to have updated document style if it is not clean and we are resolving the root element.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::resolveComputedStyle):

- Ensure the style scope is flushed so stylesheet data is current.
- Don't bail out when encountering display:none subtree, the ancestors may still affect its style.
- Fall back to a full style update if we encounter a query container with invalid style in the ancestor chain.

Canonical link: https://commits.webkit.org/267786@main
  • Loading branch information
anttijk committed Sep 8, 2023
1 parent 53a28d9 commit 514d0ac
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Test passes if there is a filled green square.


FAIL Initially display:none, not focusable assert_not_equals: got disallowed value Element node <div id="target" tabindex="1"></div>
PASS Initially display:none, not focusable
PASS Focusable after container size change

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Test passes if there is a filled green square.


FAIL Initially display:none, not focusable assert_not_equals: got disallowed value Element node <div id="target" tabindex="1"></div>
PASS Initially display:none, not focusable
PASS Focusable after container size change

6 changes: 6 additions & 0 deletions Source/WebCore/dom/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2339,6 +2339,12 @@ std::unique_ptr<RenderStyle> Document::styleForElementIgnoringPendingStylesheets
ASSERT(pseudoElementSpecifier == PseudoId::None || parentStyle);
ASSERT(Style::postResolutionCallbacksAreSuspended());

std::optional<RenderStyle> updatedDocumentStyle;
if (!parentStyle && m_needsFullStyleRebuild) {
updatedDocumentStyle.emplace(Style::resolveForDocument(*this));
parentStyle = &*updatedDocumentStyle;
}

SetForScope change(m_ignorePendingStylesheets, true);
auto& resolver = element.styleResolver();

Expand Down
18 changes: 13 additions & 5 deletions Source/WebCore/dom/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3965,6 +3965,8 @@ const RenderStyle* Element::resolveComputedStyle(ResolveComputedStyleMode mode)
{
ASSERT(isConnected());

document().styleScope().flushPendingUpdate();

bool isInDisplayNoneTree = false;

// Traverse the ancestor chain to find the rootmost element that has invalid computed style.
Expand Down Expand Up @@ -3993,17 +3995,18 @@ const RenderStyle* Element::resolveComputedStyle(ResolveComputedStyleMode mode)
}
if (mode == ResolveComputedStyleMode::RenderedOnly && existing->display() == DisplayType::None) {
isInDisplayNoneTree = true;
return nullptr;
// Invalid ancestor style may still affect this display:none style.
rootmost = nullptr;
}
}
return rootmost;
}();

if (isInDisplayNoneTree)
return nullptr;

if (!rootmostInvalidElement)
if (!rootmostInvalidElement) {
if (isInDisplayNoneTree)
return nullptr;
return existingComputedStyle();
}

auto* ancestorWithValidStyle = rootmostInvalidElement->parentElementInComposedTree();

Expand All @@ -4020,6 +4023,11 @@ const RenderStyle* Element::resolveComputedStyle(ResolveComputedStyleMode mode)
// FIXME: This is not as efficient as it could be. For example if an ancestor has a non-inherited style change but
// the styles are otherwise clean we would not need to re-resolve descendants.
for (auto& element : makeReversedRange(elementsRequiringComputedStyle)) {
if (computedStyle && computedStyle->containerType() != ContainerType::Normal) {
// If we find a query container we need to bail out and do full style update to resolve it.
if (document().updateStyleIfNeeded())
return this->computedStyle();
};
auto style = document().styleForElementIgnoringPendingStylesheets(*element, computedStyle);
computedStyle = style.get();
ElementRareData& rareData = element->ensureElementRareData();
Expand Down

0 comments on commit 514d0ac

Please sign in to comment.