Skip to content

Commit 23e800a

Browse files
alicealice
committed
Address review comments
1 parent fe0cce5 commit 23e800a

7 files changed

+59
-91
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

+6-13
Original file line numberDiff line numberDiff line change
@@ -3252,15 +3252,8 @@ String AccessibilityObject::popupValue() const
32523252

32533253
bool AccessibilityObject::hasDatalist() const
32543254
{
3255-
auto datalistId = getAttribute(listAttr);
3256-
if (datalistId.isEmpty())
3257-
return false;
3258-
3259-
auto element = this->element();
3260-
if (!element || !is<HTMLInputElement>(element))
3261-
return false;
3262-
3263-
return dynamicDowncast<HTMLInputElement>(element)->hasDataList();
3255+
RefPtr input = dynamicDowncast<HTMLInputElement>(element());
3256+
return input && input->hasDataList();
32643257
}
32653258

32663259
bool AccessibilityObject::supportsSetSize() const
@@ -4051,11 +4044,11 @@ Vector<Ref<Element>> AccessibilityObject::elementsFromAttribute(const QualifiedN
40514044
return elementsFromAttribute.value();
40524045
}
40534046

4054-
if (auto* defaultARIA = element->customElementDefaultARIAIfExists()) {
4055-
return defaultARIA->elementsForAttribute(*element, attribute);
4056-
}
4047+
if (auto* defaultARIA = element->customElementDefaultARIAIfExists()) {
4048+
return defaultARIA->elementsForAttribute(*element, attribute);
4049+
}
40574050

4058-
return { };
4051+
return { };
40594052
}
40604053

40614054
#if PLATFORM(COCOA)

‎Source/WebCore/dom/Element.cpp

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

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

23352335
if (LIKELY(startElement.isInTreeScope()))
@@ -2366,7 +2366,7 @@ RefPtr<Element> Element::getElementForAttributeInternal(const QualifiedName& att
23662366
}
23672367

23682368
if (!hasExplicitlySetElement) {
2369-
const AtomString& id = getAttribute(attributeName);
2369+
const AtomString& id = getAttribute(attributeName);
23702370
element = getElementByIdInternalIncludingDisconnected(*this, id);
23712371
}
23722372

@@ -2379,16 +2379,13 @@ RefPtr<Element> Element::getElementForAttributeInternal(const QualifiedName& att
23792379
RefPtr<Element> Element::getElementAttributeForBindings(const QualifiedName& attributeName) const
23802380
{
23812381
ASSERT(isElementReflectionAttribute(document().settings(), attributeName));
2382-
RefPtr<Element> element = getElementForAttributeInternal(attributeName);
2382+
RefPtr element = getElementForAttributeInternal(attributeName);
23832383

23842384
if (!element)
23852385
return nullptr;
23862386

2387-
if (document().settings().shadowRootReferenceTargetEnabled()) {
2388-
Ref<Node> retargeted = treeScope().retargetToScope(*element);
2389-
ASSERT(retargeted->isElementNode());
2390-
return dynamicDowncast<Element>(retargeted);
2391-
}
2387+
if (document().settings().shadowRootReferenceTargetEnabled())
2388+
return downcast<Element>(treeScope().retargetToScope(*element));
23922389

23932390
return element;
23942391
}
@@ -2446,10 +2443,9 @@ std::optional<Vector<Ref<Element>>> Element::getElementsArrayForAttributeInterna
24462443

24472444
if (document().settings().shadowRootReferenceTargetEnabled()) {
24482445
elements = compactMap(elements.value(), [&](Ref<Element>& element) -> std::optional<Ref<Element>> {
2449-
RefPtr<Element> deepReferenceTarget = element->deepShadowRootReferenceTargetOrSelf();
2450-
if (!deepReferenceTarget)
2451-
return std::nullopt;
2452-
return *deepReferenceTarget;
2446+
if (RefPtr deepReferenceTarget = element->deepShadowRootReferenceTargetOrSelf())
2447+
return *deepReferenceTarget;
2448+
return std::nullopt;
24532449
});
24542450
}
24552451

@@ -2466,9 +2462,7 @@ std::optional<Vector<Ref<Element>>> Element::getElementsArrayAttributeForBinding
24662462

24672463
if (document().settings().shadowRootReferenceTargetEnabled()) {
24682464
elements = compactMap(elements.value(), [&](Ref<Element>& element) -> std::optional<Ref<Element>> {
2469-
Ref<Node> retargeted = treeScope().retargetToScope(element);
2470-
ASSERT(retargeted->isElementNode());
2471-
return *dynamicDowncast<Element>(retargeted);
2465+
return downcast<Element>(treeScope().retargetToScope(element));
24722466
});
24732467
}
24742468

@@ -3261,8 +3255,7 @@ ExceptionOr<ShadowRoot&> Element::attachShadow(const ShadowRootInit& init)
32613255
init.serializable ? ShadowRoot::Serializable::Yes : ShadowRoot::Serializable::No,
32623256
isPrecustomizedOrDefinedCustomElement() ? ShadowRoot::AvailableToElementInternals::Yes : ShadowRoot::AvailableToElementInternals::No,
32633257
WTFMove(registry), init.registry ? ShadowRoot::ScopedCustomElementRegistry::Yes : ShadowRoot::ScopedCustomElementRegistry::No);
3264-
if (document().settings().shadowRootReferenceTargetEnabled())
3265-
shadow->setReferenceTarget(AtomString(init.referenceTarget));
3258+
shadow->setReferenceTarget(AtomString(init.referenceTarget));
32663259
addShadowRoot(shadow.copyRef());
32673260
return shadow.get();
32683261
}
@@ -3310,12 +3303,10 @@ RefPtr<const Element> Element::deepShadowRootReferenceTargetOrSelf() const {
33103303

33113304
RefPtr<const Element> element = this;
33123305

3313-
ShadowRoot* shadow = shadowRoot();
3306+
RefPtr shadow = shadowRoot();
33143307
while (shadow && shadow->hasReferenceTarget()) {
33153308
element = shadow->referenceTargetElementOrHost();
3316-
if (!element)
3317-
return nullptr;
3318-
shadow = element->shadowRoot();
3309+
shadow = element ? element->shadowRoot() : nullptr;
33193310
}
33203311
return element;
33213312
}

‎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)