Skip to content

GitLab

T
tangled
Commit 41bffb13 authored by sigbjornf's avatar sigbjornf Committed by Commit bot
Browse files
Options

Precisely account for required buttons in a radio group. 

As part of processing the name attribute for a radio button, it is
added to the current radio button group. For buttons that are
additionally "required", that leads to double accounting for the
group's count of such required buttons, as the radio button group
doesn't keep track what has been registered as "required" already
or not.

Address by having the button group track the registered "required"
state of its members/buttons.

R=keishi,tkent
BUG=

Review URL: https://codereview.chromium.org/1632493002

Cr-Commit-Position: refs/heads/master@{#371476}
parent a6c054a6
Show whitespace changes
Inline Side-by-side
Showing with 74 additions and 19 deletions
third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-remove-required-expected.txt 0 → 100644
View file @ 41bffb13
Removing a named required radio button shouldn't assert
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS document.getElementsByName("requiredRadio")[0].required is true
PASS radio.required is true
PASS radio.required is true
PASS successfullyParsed is true
TEST COMPLETE
third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-remove-required.html 0 → 100644
View file @ 41bffb13
<!DOCTYPE html>
<html>
<head>
<script src="../../../resources/js-test.js"></script>
</head>
<body>
<form>
<input type=radio name=requiredRadio id=rbutton required>
</form>
<script>
description('Removing a named required radio button shouldn\'t assert');
var radio = document.getElementById('rbutton');
shouldBeTrue('document.getElementsByName("requiredRadio")[0].required');
shouldBeTrue('radio.required');
radio.parentElement.removeChild(radio);
shouldBeTrue('radio.required');
</script>
</body>
</html>
third_party/WebKit/Source/core/html/forms/RadioButtonGroupScope.cpp
View file @ 41bffb13
...@@ -22,7 +22,7 @@ ...@@ -22,7 +22,7 @@
#include "core/InputTypeNames.h" #include "core/InputTypeNames.h"
#include "core/html/HTMLInputElement.h" #include "core/html/HTMLInputElement.h"
#include "wtf/HashSet.h" #include "wtf/HashMap.h"
namespace blink { namespace blink {
...@@ -47,7 +47,18 @@ private: ...@@ -47,7 +47,18 @@ private:
bool isValid() const; bool isValid() const;
void setCheckedButton(HTMLInputElement*); void setCheckedButton(HTMLInputElement*);
WillBeHeapHashSet<RawPtrWillBeMember<HTMLInputElement>> m_members; // The map records the 'required' state of each (button) element.
using Members = WillBeHeapHashMap<RawPtrWillBeMember<HTMLInputElement>, bool>;
#if ENABLE(OILPAN)
using MemberKeyValue = WTF::KeyValuePair<Member<HTMLInputElement>, bool>;
#else
using MemberKeyValue = WTF::KeyValuePair<HTMLInputElement*, bool>;
#endif
void updateRequiredButton(MemberKeyValue&, bool isRequired);
Members m_members;
RawPtrWillBeMember<HTMLInputElement> m_checkedButton; RawPtrWillBeMember<HTMLInputElement> m_checkedButton;
size_t m_requiredCount; size_t m_requiredCount;
}; };
...@@ -78,14 +89,28 @@ void RadioButtonGroup::setCheckedButton(HTMLInputElement* button) ...@@ -78,14 +89,28 @@ void RadioButtonGroup::setCheckedButton(HTMLInputElement* button)
oldCheckedButton->setChecked(false); oldCheckedButton->setChecked(false);
} }
void RadioButtonGroup::updateRequiredButton(MemberKeyValue& it, bool isRequired)
{
if (it.value == isRequired)
return;
it.value = isRequired;
if (isRequired) {
m_requiredCount++;
} else {
ASSERT(m_requiredCount);
m_requiredCount--;
}
}
void RadioButtonGroup::add(HTMLInputElement* button) void RadioButtonGroup::add(HTMLInputElement* button)
{ {
ASSERT(button->type() == InputTypeNames::radio); ASSERT(button->type() == InputTypeNames::radio);
if (!m_members.add(button).isNewEntry) auto addResult = m_members.add(button, false);
if (!addResult.isNewEntry)
return; return;
bool groupWasValid = isValid(); bool groupWasValid = isValid();
if (button->isRequired()) updateRequiredButton(*addResult.storedValue, button->isRequired());
++m_requiredCount;
if (button->checked()) if (button->checked())
setCheckedButton(button); setCheckedButton(button);
...@@ -112,7 +137,8 @@ void RadioButtonGroup::updateCheckedState(HTMLInputElement* button) ...@@ -112,7 +137,8 @@ void RadioButtonGroup::updateCheckedState(HTMLInputElement* button)
} }
if (wasValid != isValid()) if (wasValid != isValid())
setNeedsValidityCheckForAllButtons(); setNeedsValidityCheckForAllButtons();
for (HTMLInputElement* const inputElement : m_members) { for (auto& member : m_members) {
HTMLInputElement* const inputElement = member.key;
inputElement->pseudoStateChanged(CSSSelector::PseudoIndeterminate); inputElement->pseudoStateChanged(CSSSelector::PseudoIndeterminate);
} }
} }
...@@ -120,14 +146,12 @@ void RadioButtonGroup::updateCheckedState(HTMLInputElement* button) ...@@ -120,14 +146,12 @@ void RadioButtonGroup::updateCheckedState(HTMLInputElement* button)
void RadioButtonGroup::requiredAttributeChanged(HTMLInputElement* button) void RadioButtonGroup::requiredAttributeChanged(HTMLInputElement* button)
{ {
ASSERT(button->type() == InputTypeNames::radio); ASSERT(button->type() == InputTypeNames::radio);
ASSERT(m_members.contains(button)); auto it = m_members.find(button);
ASSERT(it != m_members.end());
bool wasValid = isValid(); bool wasValid = isValid();
if (button->isRequired()) { // Synchronize the 'required' flag for the button, along with
++m_requiredCount; // updating the overall count.
} else { updateRequiredButton(*it, button->isRequired());
ASSERT(m_requiredCount);
--m_requiredCount;
}
if (wasValid != isValid()) if (wasValid != isValid())
setNeedsValidityCheckForAllButtons(); setNeedsValidityCheckForAllButtons();
} }
...@@ -135,15 +159,13 @@ void RadioButtonGroup::requiredAttributeChanged(HTMLInputElement* button) ...@@ -135,15 +159,13 @@ void RadioButtonGroup::requiredAttributeChanged(HTMLInputElement* button)
void RadioButtonGroup::remove(HTMLInputElement* button) void RadioButtonGroup::remove(HTMLInputElement* button)
{ {
ASSERT(button->type() == InputTypeNames::radio); ASSERT(button->type() == InputTypeNames::radio);
WillBeHeapHashSet<RawPtrWillBeMember<HTMLInputElement>>::iterator it = m_members.find(button); auto it = m_members.find(button);
if (it == m_members.end()) if (it == m_members.end())
return; return;
bool wasValid = isValid(); bool wasValid = isValid();
ASSERT(it->value == button->isRequired());
updateRequiredButton(*it, false);
m_members.remove(it); m_members.remove(it);
if (button->isRequired()) {
ASSERT(m_requiredCount);
--m_requiredCount;
}
if (m_checkedButton == button) if (m_checkedButton == button)
m_checkedButton = nullptr; m_checkedButton = nullptr;
...@@ -162,7 +184,8 @@ void RadioButtonGroup::remove(HTMLInputElement* button) ...@@ -162,7 +184,8 @@ void RadioButtonGroup::remove(HTMLInputElement* button)
void RadioButtonGroup::setNeedsValidityCheckForAllButtons() void RadioButtonGroup::setNeedsValidityCheckForAllButtons()
{ {
for (HTMLInputElement* const button : m_members) { for (auto& element : m_members) {
HTMLInputElement* const button = element.key;
ASSERT(button->type() == InputTypeNames::radio); ASSERT(button->type() == InputTypeNames::radio);
button->setNeedsValidityCheck(); button->setNeedsValidityCheck();
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment