Title: [288033] trunk/Source/WebCore
- Revision
- 288033
- Author
- cdu...@apple.com
- Date
- 2022-01-14 14:46:39 -0800 (Fri, 14 Jan 2022)
Log Message
Clarify / Optimize <select> logic given that deeply nested <option> or <optgroup> are not supported
https://bugs.webkit.org/show_bug.cgi?id=235247
Reviewed by Geoffrey Garen.
Clarify / Optimize <select> logic given that deeply nested <option> or <optgroup> are not supported, as per the
specification.
An <option> is only associated with a <select> element if it is either a child of the <select> or a child of an
<optgroup> that is itself a child of the <select>:
- https://html.spec.whatwg.org/multipage/form-elements.html#concept-select-option-list
As a result, an <optgroup> is only associated with a <select> element if it is a child of that <select>.
No new tests, no Web-facing behavior change.
* html/HTMLOptGroupElement.cpp:
(WebCore::HTMLOptGroupElement::recalcSelectOptions):
(WebCore::HTMLOptGroupElement::ownerSelectElement const):
(WebCore::HTMLOptGroupElement::accessKeyAction):
* html/HTMLOptionElement.cpp:
(WebCore::HTMLOptionElement::setText):
(WebCore::HTMLOptionElement::accessKeyAction):
(WebCore::HTMLOptionElement::index const):
(WebCore::HTMLOptionElement::selected const):
(WebCore::HTMLOptionElement::setSelected):
(WebCore::HTMLOptionElement::childrenChanged):
(WebCore::HTMLOptionElement::ownerSelectElement const):
(WebCore::HTMLOptionElement::textIndentedToRespectGroupLabel const):
(WebCore::HTMLOptionElement::insertedIntoAncestor):
(WebCore::HTMLOptionElement::collectOptionInnerText const):
* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::recalcListItems const):
(WebCore::HTMLSelectElement::listBoxDefaultEventHandler):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (288032 => 288033)
--- trunk/Source/WebCore/ChangeLog 2022-01-14 22:23:35 UTC (rev 288032)
+++ trunk/Source/WebCore/ChangeLog 2022-01-14 22:46:39 UTC (rev 288033)
@@ -1,3 +1,40 @@
+2022-01-14 Chris Dumez <cdu...@apple.com>
+
+ Clarify / Optimize <select> logic given that deeply nested <option> or <optgroup> are not supported
+ https://bugs.webkit.org/show_bug.cgi?id=235247
+
+ Reviewed by Geoffrey Garen.
+
+ Clarify / Optimize <select> logic given that deeply nested <option> or <optgroup> are not supported, as per the
+ specification.
+
+ An <option> is only associated with a <select> element if it is either a child of the <select> or a child of an
+ <optgroup> that is itself a child of the <select>:
+ - https://html.spec.whatwg.org/multipage/form-elements.html#concept-select-option-list
+
+ As a result, an <optgroup> is only associated with a <select> element if it is a child of that <select>.
+
+ No new tests, no Web-facing behavior change.
+
+ * html/HTMLOptGroupElement.cpp:
+ (WebCore::HTMLOptGroupElement::recalcSelectOptions):
+ (WebCore::HTMLOptGroupElement::ownerSelectElement const):
+ (WebCore::HTMLOptGroupElement::accessKeyAction):
+ * html/HTMLOptionElement.cpp:
+ (WebCore::HTMLOptionElement::setText):
+ (WebCore::HTMLOptionElement::accessKeyAction):
+ (WebCore::HTMLOptionElement::index const):
+ (WebCore::HTMLOptionElement::selected const):
+ (WebCore::HTMLOptionElement::setSelected):
+ (WebCore::HTMLOptionElement::childrenChanged):
+ (WebCore::HTMLOptionElement::ownerSelectElement const):
+ (WebCore::HTMLOptionElement::textIndentedToRespectGroupLabel const):
+ (WebCore::HTMLOptionElement::insertedIntoAncestor):
+ (WebCore::HTMLOptionElement::collectOptionInnerText const):
+ * html/HTMLSelectElement.cpp:
+ (WebCore::HTMLSelectElement::recalcListItems const):
+ (WebCore::HTMLSelectElement::listBoxDefaultEventHandler):
+
2022-01-14 Jer Noble <jer.no...@apple.com>
[Cocoa] rVFC() isn't called for initial video load
Modified: trunk/Source/WebCore/html/HTMLOptGroupElement.cpp (288032 => 288033)
--- trunk/Source/WebCore/html/HTMLOptGroupElement.cpp 2022-01-14 22:23:35 UTC (rev 288032)
+++ trunk/Source/WebCore/html/HTMLOptGroupElement.cpp 2022-01-14 22:46:39 UTC (rev 288033)
@@ -102,7 +102,7 @@
void HTMLOptGroupElement::recalcSelectOptions()
{
- if (RefPtr selectElement = ancestorsOfType<HTMLSelectElement>(*this).first()) {
+ if (RefPtr selectElement = ownerSelectElement()) {
selectElement->setRecalcListItems();
selectElement->updateValidity();
}
@@ -122,12 +122,13 @@
HTMLSelectElement* HTMLOptGroupElement::ownerSelectElement() const
{
- return const_cast<HTMLSelectElement*>(ancestorsOfType<HTMLSelectElement>(*this).first());
+ auto* parent = parentNode();
+ return is<HTMLSelectElement>(parent) ? downcast<HTMLSelectElement>(parent) : nullptr;
}
bool HTMLOptGroupElement::accessKeyAction(bool)
{
- RefPtr<HTMLSelectElement> select = ownerSelectElement();
+ RefPtr select = ownerSelectElement();
// send to the parent to bring focus to the list box
if (select && !select->focused())
return select->accessKeyAction(false);
Modified: trunk/Source/WebCore/html/HTMLOptionElement.cpp (288032 => 288033)
--- trunk/Source/WebCore/html/HTMLOptionElement.cpp 2022-01-14 22:23:35 UTC (rev 288032)
+++ trunk/Source/WebCore/html/HTMLOptionElement.cpp 2022-01-14 22:46:39 UTC (rev 288033)
@@ -114,17 +114,17 @@
void HTMLOptionElement::setText(const String &text)
{
- Ref<HTMLOptionElement> protectedThis(*this);
+ Ref protectedThis { *this };
// Changing the text causes a recalc of a select's items, which will reset the selected
// index to the first item if the select is single selection with a menu list. We attempt to
// preserve the selected item.
- RefPtr<HTMLSelectElement> select = ownerSelectElement();
+ RefPtr select = ownerSelectElement();
bool selectIsMenuList = select && select->usesMenuList();
int oldSelectedIndex = selectIsMenuList ? select->selectedIndex() : -1;
// Handle the common special case where there's exactly 1 child node, and it's a text node.
- RefPtr<Node> child = firstChild();
+ RefPtr child = firstChild();
if (is<Text>(child) && !child->nextSibling())
downcast<Text>(*child).setData(text);
else {
@@ -138,7 +138,7 @@
bool HTMLOptionElement::accessKeyAction(bool)
{
- RefPtr<HTMLSelectElement> select = ownerSelectElement();
+ RefPtr select = ownerSelectElement();
if (select) {
select->accessKeySetSelectedIndex(index());
return true;
@@ -150,7 +150,7 @@
{
// It would be faster to cache the index, but harder to get it right in all cases.
- RefPtr<HTMLSelectElement> selectElement = ownerSelectElement();
+ RefPtr selectElement = ownerSelectElement();
if (!selectElement)
return 0;
@@ -212,7 +212,7 @@
bool HTMLOptionElement::selected() const
{
- if (RefPtr<HTMLSelectElement> select = ownerSelectElement())
+ if (RefPtr select = ownerSelectElement())
select->updateListItemSelectedStates();
return m_isSelected;
}
@@ -224,7 +224,7 @@
setSelectedState(selected);
- if (RefPtr<HTMLSelectElement> select = ownerSelectElement())
+ if (RefPtr select = ownerSelectElement())
select->optionSelectionStateChanged(*this, selected);
}
@@ -249,7 +249,7 @@
for (auto& dataList : ancestorsOfType<HTMLDataListElement>(*this))
dataList.optionElementChildrenChanged();
#endif
- if (RefPtr<HTMLSelectElement> select = ownerSelectElement())
+ if (RefPtr select = ownerSelectElement())
select->optionElementChildrenChanged();
HTMLElement::childrenChanged(change);
}
@@ -256,7 +256,13 @@
HTMLSelectElement* HTMLOptionElement::ownerSelectElement() const
{
- return const_cast<HTMLSelectElement*>(ancestorsOfType<HTMLSelectElement>(*this).first());
+ if (auto* parent = parentElement()) {
+ if (is<HTMLSelectElement>(*parent))
+ return downcast<HTMLSelectElement>(parent);
+ if (is<HTMLOptGroupElement>(*parent))
+ return downcast<HTMLOptGroupElement>(*parent).ownerSelectElement();
+ }
+ return nullptr;
}
String HTMLOptionElement::label() const
@@ -292,7 +298,7 @@
String HTMLOptionElement::textIndentedToRespectGroupLabel() const
{
- RefPtr<ContainerNode> parent = parentNode();
+ RefPtr parent = parentNode();
if (is<HTMLOptGroupElement>(parent))
return " " + displayLabel();
return displayLabel();
@@ -311,7 +317,7 @@
Node::InsertedIntoAncestorResult HTMLOptionElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
- if (RefPtr<HTMLSelectElement> select = ownerSelectElement()) {
+ if (RefPtr select = ownerSelectElement()) {
select->setRecalcListItems();
select->updateValidity();
// Do not call selected() since calling updateListItemSelectedStates()
@@ -329,7 +335,7 @@
String HTMLOptionElement::collectOptionInnerText() const
{
StringBuilder text;
- for (RefPtr<Node> node = firstChild(); node; ) {
+ for (RefPtr node = firstChild(); node; ) {
if (is<Text>(*node))
text.append(node->nodeValue());
// Text nodes inside script elements are not part of the option text.
Modified: trunk/Source/WebCore/html/HTMLSelectElement.cpp (288032 => 288033)
--- trunk/Source/WebCore/html/HTMLSelectElement.cpp 2022-01-14 22:23:35 UTC (rev 288032)
+++ trunk/Source/WebCore/html/HTMLSelectElement.cpp 2022-01-14 22:46:39 UTC (rev 288033)
@@ -770,50 +770,31 @@
RefPtr<HTMLOptionElement> foundSelected;
RefPtr<HTMLOptionElement> firstOption;
- for (RefPtr<Element> currentElement = ElementTraversal::firstWithin(*this); currentElement; ) {
- if (!is<HTMLElement>(*currentElement)) {
- currentElement = ElementTraversal::nextSkippingChildren(*currentElement, this);
- continue;
- }
- HTMLElement& current = downcast<HTMLElement>(*currentElement);
-
- // Only consider optgroup elements that are direct children of the select element.
- if (is<HTMLOptGroupElement>(current) && current.parentNode() == this) {
- m_listItems.append(¤t);
- if (RefPtr<Element> nextElement = ElementTraversal::firstWithin(current)) {
- currentElement = nextElement;
- continue;
+ auto handleOptionElement = [&](HTMLOptionElement& option) {
+ m_listItems.append(&option);
+ if (updateSelectedStates && !m_multiple) {
+ if (!firstOption)
+ firstOption = &option;
+ if (option.selected()) {
+ if (foundSelected)
+ foundSelected->setSelectedState(false);
+ foundSelected = &option;
+ } else if (m_size <= 1 && !foundSelected && !option.isDisabledFormControl()) {
+ foundSelected = &option;
+ foundSelected->setSelectedState(true);
}
}
+ };
- if (is<HTMLOptionElement>(current)) {
- m_listItems.append(¤t);
-
- if (updateSelectedStates && !m_multiple) {
- HTMLOptionElement& option = downcast<HTMLOptionElement>(current);
- if (!firstOption)
- firstOption = &option;
- if (option.selected()) {
- if (foundSelected)
- foundSelected->setSelectedState(false);
- foundSelected = &option;
- } else if (m_size <= 1 && !foundSelected && !option.isDisabledFormControl()) {
- foundSelected = &option;
- foundSelected->setSelectedState(true);
- }
- }
- }
-
- if (current.hasTagName(hrTag))
- m_listItems.append(¤t);
-
- // In conforming HTML code, only <optgroup> and <option> will be found
- // within a <select>. We call NodeTraversal::nextSkippingChildren so that we only step
- // into those tags that we choose to. For web-compat, we should cope
- // with the case where odd tags like a <div> have been added but we
- // handle this because such tags have already been removed from the
- // <select>'s subtree at this point.
- currentElement = ElementTraversal::nextSkippingChildren(*currentElement, this);
+ for (auto& child : childrenOfType<HTMLElement>(*const_cast<HTMLSelectElement*>(this))) {
+ if (is<HTMLOptGroupElement>(child)) {
+ m_listItems.append(&child);
+ for (auto& option : childrenOfType<HTMLOptionElement>(child))
+ handleOptionElement(option);
+ } else if (is<HTMLOptionElement>(child)) {
+ handleOptionElement(downcast<HTMLOptionElement>(child));
+ } else if (is<HTMLHRElement>(child))
+ m_listItems.append(&child);
}
if (!foundSelected && m_size <= 1 && firstOption && !firstOption->selected())
@@ -1349,7 +1330,7 @@
updateSelectedState(listIndex, mouseEvent.ctrlKey(), mouseEvent.shiftKey());
#endif
}
- if (RefPtr<Frame> frame = document().frame())
+ if (RefPtr frame = document().frame())
frame->eventHandler().setMouseDownMayStartAutoscroll();
mouseEvent.setDefaultHandled();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
