Skip to content

Commit

Permalink
imported/w3c/web-platform-tests/inert/inert-node-is-unfocusable.html …
Browse files Browse the repository at this point in the history
…is failing

https://bugs.webkit.org/show_bug.cgi?id=247803
<rdar://problem/102367494>

Reviewed by Alan Baradlay.

Ancestors may have changes that need to be inherited to the descendants. We handle this correctly for the current
chain but the ancestors invalidity bit gets cleared in the process. If computed style is later resolved for a
sibling we fail to take this into account.

* LayoutTests/platform/mac-wk2/imported/w3c/web-platform-tests/inert/inert-node-is-unfocusable-expected.txt: Removed.
LayoutTests/editing/pasteboard/copy-paste-across-shadow-boundaries-4-expected.txt:

The new results are equivalent.

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

Fix by marking ancestor computed style explicitly invalid if the parent has an inherited style change.
Also add an optimizations for the case where style is already valid.

(WebCore::Element::computedStyle):
(WebCore::Element::computedStyleForEditability):

Add a version of computedStyle() that calls resolveComputedStyle unconditionally. This should
really be done in all cases, not just for editability testing but that will be left as future work.

* Source/WebCore/dom/Element.h:
* Source/WebCore/dom/Node.cpp:
(WebCore::Node::computeEditabilityWithStyle const):

Use computedStyleForEditability().

(WebCore::Node::computedStyle):

* Source/WebCore/editing/AppendNodeCommand.cpp:
(WebCore::AppendNodeCommand::AppendNodeCommand):

Remove hasEditableStyle assert. It is tested in apply() functions and no other commands has this assert.

Canonical link: https://commits.webkit.org/256782@main
  • Loading branch information
anttijk committed Nov 17, 2022
1 parent 47042b3 commit aaa2528
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 20 deletions.
Expand Up @@ -2,9 +2,11 @@ This tests copying and pasting content across shadow boundaries.
To test manually, copy text blow starting from "hello" ending with "Web", and paste into the green box below. All the text shoul be copied & pasted.

pasted html:
| "hello "
| <span>
| style="display: contents;"
| style="font-size: 16px; -webkit-text-fill-color: rgb(0, 0, 0); -webkit-text-stroke-color: rgb(0, 0, 0);"
| "hello "
| <span>
| style="-webkit-text-fill-color: rgb(0, 0, 0); -webkit-text-stroke-color: rgb(0, 0, 0); display: contents;"
| "world Web<#selection-caret>"

text/plain:
Expand Down

This file was deleted.

24 changes: 24 additions & 0 deletions Source/WebCore/dom/Element.cpp
Expand Up @@ -3714,6 +3714,10 @@ const RenderStyle* Element::resolveComputedStyle(ResolveComputedStyleMode mode)

// Traverse the ancestor chain to find the rootmost element that has invalid computed style.
auto* rootmostInvalidElement = [&]() -> const Element* {
// In ResolveComputedStyleMode::RenderedOnly case we check for display:none ancestors.
if (mode == ResolveComputedStyleMode::Normal && !document().hasPendingStyleRecalc() && existingComputedStyle())
return nullptr;

if (document().hasPendingFullStyleRebuild())
return document().documentElement();

Expand Down Expand Up @@ -3765,6 +3769,16 @@ const RenderStyle* Element::resolveComputedStyle(ResolveComputedStyleMode mode)
auto style = document().styleForElementIgnoringPendingStylesheets(*element, computedStyle);
computedStyle = style.get();
ElementRareData& rareData = element->ensureElementRareData();
if (auto* existing = rareData.computedStyle()) {
auto change = Style::determineChange(*existing, *style);
if (change > Style::Change::NonInherited) {
for (auto& child : composedTreeChildren(*element)) {
if (!is<Element>(child))
continue;
downcast<Element>(child).setNodeFlag(NodeFlag::IsComputedStyleInvalidFlag);
}
}
}
rareData.setComputedStyle(WTFMove(style));
element->clearNodeFlag(NodeFlag::IsComputedStyleInvalidFlag);
if (hadDisplayContents && computedStyle->display() != DisplayType::Contents)
Expand Down Expand Up @@ -3825,6 +3839,7 @@ const RenderStyle* Element::computedStyle(PseudoId pseudoElementSpecifier)
if (PseudoElement* pseudoElement = beforeOrAfterPseudoElement(*this, pseudoElementSpecifier))
return pseudoElement->computedStyle();

// FIXME: This should call resolveComputedStyle() unconditionally so we check if the style is valid.
auto* style = existingComputedStyle();
if (!style)
style = resolveComputedStyle();
Expand All @@ -3838,6 +3853,15 @@ const RenderStyle* Element::computedStyle(PseudoId pseudoElementSpecifier)
return style;
}

// FIXME: The caller should be able to just use computedStyle().
const RenderStyle* Element::computedStyleForEditability()
{
if (!isConnected())
return nullptr;

return resolveComputedStyle();
}

bool Element::needsStyleInvalidation() const
{
if (!inRenderedDocument())
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/dom/Element.h
Expand Up @@ -399,6 +399,7 @@ class Element : public ContainerNode {
WEBCORE_EXPORT ExceptionOr<void> insertAdjacentText(const String& where, String&& text);

const RenderStyle* computedStyle(PseudoId = PseudoId::None) override;
const RenderStyle* computedStyleForEditability();

bool needsStyleInvalidation() const;

Expand Down
15 changes: 11 additions & 4 deletions Source/WebCore/dom/Node.cpp
Expand Up @@ -777,7 +777,7 @@ static Node::Editability computeEditabilityFromComputedStyle(const RenderStyle&
return Node::Editability::ReadOnly;
}

Node::Editability Node::computeEditabilityWithStyle(const RenderStyle* style, UserSelectAllTreatment treatment, ShouldUpdateStyle shouldUpdateStyle) const
Node::Editability Node::computeEditabilityWithStyle(const RenderStyle* incomingStyle, UserSelectAllTreatment treatment, ShouldUpdateStyle shouldUpdateStyle) const
{
if (!document().hasLivingRenderTree() || isPseudoElement())
return Editability::ReadOnly;
Expand All @@ -794,8 +794,15 @@ Node::Editability Node::computeEditabilityWithStyle(const RenderStyle* style, Us
document->updateStyleIfNeeded();
}

if (!style)
style = isDocumentNode() ? renderStyle() : const_cast<Node*>(this)->computedStyle();
auto* style = [&] {
if (incomingStyle)
return incomingStyle;
if (isDocumentNode())
return renderStyle();
auto* element = is<Element>(this) ? downcast<Element>(this) : parentElementInComposedTree();
return element ? const_cast<Element&>(*element).computedStyleForEditability() : nullptr;
}();

if (!style)
return Editability::ReadOnly;

Expand Down Expand Up @@ -1127,7 +1134,7 @@ Node* Node::pseudoAwareLastChild() const

const RenderStyle* Node::computedStyle(PseudoId pseudoElementSpecifier)
{
auto* composedParent = composedTreeAncestors(*this).first();
auto* composedParent = parentElementInComposedTree();
if (!composedParent)
return nullptr;
return composedParent->computedStyle(pseudoElementSpecifier);
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/editing/AppendNodeCommand.cpp
Expand Up @@ -41,7 +41,6 @@ AppendNodeCommand::AppendNodeCommand(Ref<ContainerNode>&& parent, Ref<Node>&& no
, m_node(WTFMove(node))
{
ASSERT(!m_node->parentNode());
ASSERT(m_parent->hasEditableStyle() || !m_parent->renderer());
}

void AppendNodeCommand::doApply()
Expand Down

0 comments on commit aaa2528

Please sign in to comment.