Skip to content

Commit e5c3a14

Browse files
alicealice
committed
Address review comments
1 parent 9a5437d commit e5c3a14

7 files changed

+55
-87
lines changed

‎Source/WebCore/accessibility/AXObjectCache.cpp

+1-4
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,8 @@ static bool nodeAndRendererAreValid(Node* node)
174174

175175
static RefPtr<Element> getElementByIdWithReferenceTarget(const TreeScope& treeScope, const AtomString& elementId)
176176
{
177-
if (elementId.isNull())
178-
return nullptr;
179-
180177
RefPtr<Element> element = treeScope.getElementById(elementId);
181-
return element->deepShadowRootReferenceTargetOrSelf();
178+
return element ? element->deepShadowRootReferenceTargetOrSelf() : nullptr;
182179
}
183180

184181
AccessibilityObjectInclusion AXComputedObjectAttributeCache::getIgnored(AXID id) const

‎Source/WebCore/accessibility/AccessibilityObject.cpp

+2-9
Original file line numberDiff line numberDiff line change
@@ -3250,15 +3250,8 @@ String AccessibilityObject::popupValue() const
32503250
bool AccessibilityObject::hasDatalist() const
32513251
{
32523252
#if ENABLE(DATALIST_ELEMENT)
3253-
auto datalistId = getAttribute(listAttr);
3254-
if (datalistId.isNull() || datalistId.isEmpty())
3255-
return false;
3256-
3257-
auto element = this->element();
3258-
if (!element || !is<HTMLInputElement>(element))
3259-
return false;
3260-
3261-
return dynamicDowncast<HTMLInputElement>(element)->hasDataList();
3253+
RefPtr input = dynamicDowncast<HTMLInputElement>(element());
3254+
return input && input->hasDataList();
32623255
#else
32633256
return false;
32643257
#endif

‎Source/WebCore/dom/Element.cpp

+12-21
Original file line numberDiff line numberDiff line change
@@ -2332,7 +2332,7 @@ ExplicitlySetAttrElementsMap* Element::explicitlySetAttrElementsMapIfExists() co
23322332

23332333
static RefPtr<Element> getElementByIdInternalIncludingDisconnected(const Element& startElement, const AtomString& id)
23342334
{
2335-
if (id.isNull() || id.isEmpty())
2335+
if (id.isEmpty())
23362336
return nullptr;
23372337

23382338
if (LIKELY(startElement.isInTreeScope()))
@@ -2369,7 +2369,7 @@ RefPtr<Element> Element::getElementForAttributeInternal(const QualifiedName& att
23692369
}
23702370

23712371
if (!hasExplicitlySetElement) {
2372-
const AtomString& id = getAttribute(attributeName);
2372+
const AtomString& id = getAttribute(attributeName);
23732373
element = getElementByIdInternalIncludingDisconnected(*this, id);
23742374
}
23752375

@@ -2382,16 +2382,13 @@ RefPtr<Element> Element::getElementForAttributeInternal(const QualifiedName& att
23822382
RefPtr<Element> Element::getElementAttributeForBindings(const QualifiedName& attributeName) const
23832383
{
23842384
ASSERT(isElementReflectionAttribute(document().settings(), attributeName));
2385-
RefPtr<Element> element = getElementForAttributeInternal(attributeName);
2385+
RefPtr element = getElementForAttributeInternal(attributeName);
23862386

23872387
if (!element)
23882388
return nullptr;
23892389

2390-
if (document().settings().shadowRootReferenceTargetEnabled()) {
2391-
Ref<Node> retargeted = treeScope().retargetToScope(*element);
2392-
ASSERT(retargeted->isElementNode());
2393-
return dynamicDowncast<Element>(retargeted);
2394-
}
2390+
if (document().settings().shadowRootReferenceTargetEnabled())
2391+
return downcast<Element>(treeScope().retargetToScope(*element));
23952392

23962393
return element;
23972394
}
@@ -2449,10 +2446,9 @@ std::optional<Vector<Ref<Element>>> Element::getElementsArrayForAttributeInterna
24492446

24502447
if (document().settings().shadowRootReferenceTargetEnabled()) {
24512448
elements = compactMap(elements.value(), [&](Ref<Element>& element) -> std::optional<Ref<Element>> {
2452-
RefPtr<Element> deepReferenceTarget = element->deepShadowRootReferenceTargetOrSelf();
2453-
if (!deepReferenceTarget)
2454-
return std::nullopt;
2455-
return *deepReferenceTarget;
2449+
if (RefPtr deepReferenceTarget = element->deepShadowRootReferenceTargetOrSelf())
2450+
return *deepReferenceTarget;
2451+
return std::nullopt;
24562452
});
24572453
}
24582454

@@ -2469,9 +2465,7 @@ std::optional<Vector<Ref<Element>>> Element::getElementsArrayAttributeForBinding
24692465

24702466
if (document().settings().shadowRootReferenceTargetEnabled()) {
24712467
elements = compactMap(elements.value(), [&](Ref<Element>& element) -> std::optional<Ref<Element>> {
2472-
Ref<Node> retargeted = treeScope().retargetToScope(element);
2473-
ASSERT(retargeted->isElementNode());
2474-
return *dynamicDowncast<Element>(retargeted);
2468+
return downcast<Element>(treeScope().retargetToScope(element));
24752469
});
24762470
}
24772471

@@ -3259,8 +3253,7 @@ ExceptionOr<ShadowRoot&> Element::attachShadow(const ShadowRootInit& init)
32593253
init.serializable ? ShadowRoot::Serializable::Yes : ShadowRoot::Serializable::No,
32603254
isPrecustomizedOrDefinedCustomElement() ? ShadowRoot::AvailableToElementInternals::Yes : ShadowRoot::AvailableToElementInternals::No,
32613255
WTFMove(registry), init.registry ? ShadowRoot::ScopedCustomElementRegistry::Yes : ShadowRoot::ScopedCustomElementRegistry::No);
3262-
if (document().settings().shadowRootReferenceTargetEnabled())
3263-
shadow->setReferenceTarget(AtomString(init.referenceTarget));
3256+
shadow->setReferenceTarget(AtomString(init.referenceTarget));
32643257
addShadowRoot(shadow.copyRef());
32653258
return shadow.get();
32663259
}
@@ -3308,12 +3301,10 @@ RefPtr<const Element> Element::deepShadowRootReferenceTargetOrSelf() const {
33083301

33093302
RefPtr<const Element> element = this;
33103303

3311-
ShadowRoot* shadow = shadowRoot();
3304+
RefPtr shadow = shadowRoot();
33123305
while (shadow && shadow->hasReferenceTarget()) {
33133306
element = shadow->referenceTargetElementOrHost();
3314-
if (!element)
3315-
return nullptr;
3316-
shadow = element->shadowRoot();
3307+
shadow = element ? element->shadowRoot() : nullptr;
33173308
}
33183309
return element;
33193310
}

‎Source/WebCore/html/FormAssociatedElement.cpp

+7-8
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,13 @@ FormAssociatedElement::FormAssociatedElement(HTMLFormElement* form)
3737

3838
HTMLFormElement* FormAssociatedElement::formForBindings() const
3939
{
40-
HTMLFormElement* formElement = form();
41-
if (!formElement)
42-
return nullptr;
43-
44-
if (asHTMLElement().document().settings().shadowRootReferenceTargetEnabled() && &formElement->treeScope() != &asHTMLElement().treeScope())
45-
return nullptr;
46-
47-
return formElement;
40+
if (asHTMLElement().document().settings().shadowRootReferenceTargetEnabled()) {
41+
auto* form = this->form();
42+
if (!form || &form->treeScope() != &asHTMLElement().treeScope())
43+
return nullptr;
44+
return form;
45+
}
46+
return form();
4847
}
4948

5049
void FormAssociatedElement::setFormInternal(RefPtr<HTMLFormElement>&& newForm)

‎Source/WebCore/html/HTMLLabelElement.cpp

+19-27
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,11 @@ RefPtr<HTMLElement> HTMLLabelElement::control() const
7979
// the form element must be "labelable form-associated element".
8080
for (const HTMLElement& descendant : descendantsOfType<HTMLElement>(*this)) {
8181
if (document().settings().shadowRootReferenceTargetEnabled()) {
82-
RefPtr<const Element> referenceTarget = descendant.deepShadowRootReferenceTargetOrSelf();
83-
if (!referenceTarget || !is<HTMLElement>(referenceTarget))
84-
continue;
85-
const HTMLElement* htmlReferenceTarget = dynamicDowncast<HTMLElement>(referenceTarget.get());
86-
if (htmlReferenceTarget->isLabelable())
87-
return const_cast<HTMLElement*>(htmlReferenceTarget);
88-
} else {
89-
if (descendant.isLabelable())
90-
return const_cast<HTMLElement*>(&descendant);
82+
RefPtr referenceTarget = dynamicDowncast<const HTMLElement>(descendant.deepShadowRootReferenceTargetOrSelf());
83+
if (referenceTarget && referenceTarget->isLabelable())
84+
return const_cast<HTMLElement*>(referenceTarget.get());
85+
} else if (descendant.isLabelable()) {
86+
return const_cast<HTMLElement*>(&descendant);
9187
}
9288
}
9389
return nullptr;
@@ -97,18 +93,16 @@ RefPtr<HTMLElement> HTMLLabelElement::control() const
9793

9894
RefPtr<HTMLElement> HTMLLabelElement::controlForBindings() const
9995
{
100-
RefPtr<HTMLElement> controlElement = control();
101-
102-
if (!controlElement)
103-
return nullptr;
104-
10596
if (document().settings().shadowRootReferenceTargetEnabled()) {
106-
Ref<Node> retargeted = treeScope().retargetToScope(*controlElement);
107-
ASSERT(retargeted->isHTMLElement());
108-
controlElement = dynamicDowncast<HTMLElement>(retargeted);
97+
RefPtr<HTMLElement> control = this->control();
98+
if (!control)
99+
return nullptr;
100+
101+
Ref<Node> retargeted = treeScope().retargetToScope(*control);
102+
return downcast<HTMLElement>(retargeted);
109103
}
110104

111-
return controlElement;
105+
return control();
112106
}
113107

114108
HTMLFormElement* HTMLLabelElement::form() const
@@ -122,15 +116,13 @@ HTMLFormElement* HTMLLabelElement::form() const
122116

123117
HTMLFormElement* HTMLLabelElement::formForBindings() const
124118
{
125-
HTMLFormElement* formElement = form();
126-
127-
if (!formElement)
128-
return nullptr;
129-
130-
if (document().settings().shadowRootReferenceTargetEnabled() && &formElement->treeScope() != &treeScope())
131-
return nullptr;
132-
133-
return formElement;
119+
if (document().settings().shadowRootReferenceTargetEnabled()) {
120+
auto* form = this->form();
121+
if (!form || &form->treeScope() != &treeScope())
122+
return nullptr;
123+
return form;
124+
}
125+
return form();
134126
}
135127

136128
void HTMLLabelElement::setActive(bool down, Style::InvalidationScope invalidationScope)

‎Source/WebCore/html/HTMLLegendElement.cpp

+7-9
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,13 @@ HTMLFormElement* HTMLLegendElement::form() const
5858

5959
HTMLFormElement* HTMLLegendElement::formForBindings() const
6060
{
61-
HTMLFormElement* formElement = form();
62-
63-
if (!formElement)
64-
return nullptr;
65-
66-
if (document().settings().shadowRootReferenceTargetEnabled() && &formElement->treeScope() != &treeScope())
67-
return nullptr;
68-
69-
return formElement;
61+
if (document().settings().shadowRootReferenceTargetEnabled()) {
62+
auto* form = this->form();
63+
if (!form || &form->treeScope() != &treeScope())
64+
return nullptr;
65+
return form;
66+
}
67+
return form();
7068
}
7169

7270
} // namespace

‎Source/WebCore/html/HTMLOptionElement.cpp

+7-9
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,13 @@ HTMLFormElement* HTMLOptionElement::form() const
147147

148148
HTMLFormElement* HTMLOptionElement::formForBindings() const
149149
{
150-
HTMLFormElement* formElement = form();
151-
152-
if (!formElement)
153-
return nullptr;
154-
155-
if (document().settings().shadowRootReferenceTargetEnabled() && &formElement->treeScope() != &treeScope())
156-
return nullptr;
157-
158-
return formElement;
150+
if (document().settings().shadowRootReferenceTargetEnabled()) {
151+
auto* form = this->form();
152+
if (!form || &form->treeScope() != &treeScope())
153+
return nullptr;
154+
return form;
155+
}
156+
return form();
159157
}
160158

161159
int HTMLOptionElement::index() const

0 commit comments

Comments
 (0)