Skip to content

Commit

Permalink
AX: Input time elements with no value are read incorrectly by VoiceOver.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=271394
<rdar://problem/125176285>

Reviewed by Tyler Wilcock.

The problem occurs because BaseDateAndTimeInputType::valueAsDouble returns NaN if the value attribute is not present. Thus, accessibilityValueAsDate was returning a bogus WallTime. This patch fixes the problem by getting the placeholder value set during the creation of the shadow subtree for the datetime element, which is the value shown on the screen.

* LayoutTests/accessibility/datetime/input-time-label-value-expected.txt:
* LayoutTests/accessibility/datetime/input-time-label-value.html:
Added a test case with no value.

* LayoutTests/platform/ios/accessibility/datetime/input-time-label-value-expected.txt:
* LayoutTests/platform/mac-ventura/accessibility/datetime/input-time-label-value-expected.txt:
* Source/WebCore/html/BaseDateAndTimeInputType.cpp:
(WebCore::BaseDateAndTimeInputType::accessibilityValueAsDate const):
(WebCore::BaseDateAndTimeInputType::setupDateTimeChooserParameters):
* Source/WebCore/html/shadow/DateTimeEditElement.cpp:
(WebCore::DateTimeEditElement::placeholderValue const):
(WebCore::DateTimeEditElement::valueAsDateTimeFieldsState const):
* Source/WebCore/html/shadow/DateTimeEditElement.h:
* Source/WebCore/html/shadow/DateTimeFieldElement.h:
* Source/WebCore/html/shadow/DateTimeFieldElements.cpp:
(WebCore::DateTimeDayFieldElement::populateDateTimeFieldsState):
(WebCore::DateTimeHourFieldElement::populateDateTimeFieldsState):
(WebCore::DateTimeMeridiemFieldElement::populateDateTimeFieldsState):
(WebCore::DateTimeMillisecondFieldElement::populateDateTimeFieldsState):
(WebCore::DateTimeMinuteFieldElement::populateDateTimeFieldsState):
(WebCore::DateTimeMonthFieldElement::populateDateTimeFieldsState):
(WebCore::DateTimeSecondFieldElement::populateDateTimeFieldsState):
(WebCore::DateTimeSymbolicMonthFieldElement::populateDateTimeFieldsState):
(WebCore::DateTimeYearFieldElement::populateDateTimeFieldsState):
Modified the populateDateTimeFieldsState method to return the placeholder value if the value has not been set.

* Source/WebCore/html/shadow/DateTimeFieldElements.h:
* Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp:
(WebCore::DateTimeNumericFieldElement::DateTimeNumericFieldElement):
(WebCore::DateTimeNumericFieldElement::valueAsInteger const): Inlined in header.
* Source/WebCore/html/shadow/DateTimeNumericFieldElement.h:
* Source/WebCore/html/shadow/DateTimeSymbolicFieldElement.cpp:
(WebCore::DateTimeSymbolicFieldElement::valueAsInteger const): Inlined in header.
* Source/WebCore/html/shadow/DateTimeSymbolicFieldElement.h:

Canonical link: https://commits.webkit.org/276626@main
  • Loading branch information
AndresGonzalezApple committed Mar 25, 2024
1 parent 5ce30e7 commit f2989cc
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ inputtime4:
PASS: inputtime.title === 'AXTitle: '
AXValue: 3:45 PM
PASS: inputtime.dateValue === 'AXDateValue: 1970-01-01 15:45:12 +0000'
inputtime5:
PASS: inputtime.title === 'AXTitle: '
AXValue:
PASS: inputtime.dateValue === 'AXDateValue: 1970-01-01 12:30:00 +0000'

PASS successfullyParsed is true

Expand Down
11 changes: 11 additions & 0 deletions LayoutTests/accessibility/datetime/input-time-label-value.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<label for="inputtime2">Meeting time:</label><input type="time" id="inputtime2" min="07:00" max="09:00" step="300" placeholder="00:00 AM" value="08:30" />
<label><input type="time" id="inputtime3" min="07:00" max="09:00" step="300" placeholder="00:00 AM" value="08:30">Meeting time:</label>
<input type="time" id="inputtime4" min="07:00" max="09:00" step="300" value="15:45:12">
<input type="time" id="inputtime5" />

<script>
let output = "This tests input type=time label and value properties.\n\n";
Expand Down Expand Up @@ -41,6 +42,16 @@
else
output += "\n";

// Case 5: no label, no value.
inputtime = accessibilityController.accessibleElementById("inputtime5");
output += "inputtime5:\n";
output += expect("inputtime.title", "'AXTitle: '");
output += `${inputtime.stringValue}\n`;
if (platform == "mac")
output += expect("inputtime.dateValue", "'AXDateValue: 1970-01-01 12:30:00 +0000'");
else
output += "\n";

debug(output);
}
</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ inputtime4:
PASS: inputtime.title === 'AXTitle: '
AXValue: 3:45 PM

inputtime5:
PASS: inputtime.title === 'AXTitle: '
AXValue:


PASS successfullyParsed is true

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ inputtime4:
PASS: inputtime.title === 'AXTitle: '
AXValue: 3:45 PM
PASS: inputtime.dateValue === 'AXDateValue: 1970-01-01 15:45:12 +0000'
inputtime5:
PASS: inputtime.title === 'AXTitle: '
AXValue:
PASS: inputtime.dateValue === 'AXDateValue: 1970-01-01 12:30:00 +0000'

PASS successfullyParsed is true

Expand Down
21 changes: 19 additions & 2 deletions Source/WebCore/html/BaseDateAndTimeInputType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,25 @@ ExceptionOr<void> BaseDateAndTimeInputType::setValueAsDate(WallTime value) const

WallTime BaseDateAndTimeInputType::accessibilityValueAsDate() const
{
return WallTime::fromRawSeconds(Seconds::fromMilliseconds(valueAsDouble()).value());
double dateAsDouble = valueAsDouble();
if (std::isnan(dateAsDouble) && m_dateTimeEditElement) {
// The value for this element has not been set. Try to get a value from
// m_dateTimeEditElement if exists. That value may have been indirectly
// set as placeholder values for the field elements.
String value = m_dateTimeEditElement->value();
if (value.isEmpty())
value = m_dateTimeEditElement->placeholderValue();
if (value.isEmpty())
return { };

auto decimal = parseToNumber(value, Decimal::nan());
if (decimal.isFinite())
dateAsDouble = decimal.toDouble();
}

if (std::isnan(dateAsDouble))
return { };
return WallTime::fromRawSeconds(Seconds::fromMilliseconds(dateAsDouble).value());
}

double BaseDateAndTimeInputType::valueAsDouble() const
Expand Down Expand Up @@ -601,7 +619,6 @@ bool BaseDateAndTimeInputType::setupDateTimeChooserParameters(DateTimeChooserPar
auto* computedStyle = element.computedStyle();
parameters.isAnchorElementRTL = computedStyle->direction() == TextDirection::RTL;
parameters.useDarkAppearance = document.useDarkAppearance(computedStyle);

auto date = valueOrDefault(parseToDateComponents(element.value()));
parameters.hasSecondField = shouldHaveSecondField(date);
parameters.hasMillisecondField = shouldHaveMillisecondField(date);
Expand Down
9 changes: 7 additions & 2 deletions Source/WebCore/html/shadow/DateTimeEditElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -404,11 +404,16 @@ String DateTimeEditElement::value() const
return m_editControlOwner ? m_editControlOwner->formatDateTimeFieldsState(valueAsDateTimeFieldsState()) : emptyString();
}

DateTimeFieldsState DateTimeEditElement::valueAsDateTimeFieldsState() const
String DateTimeEditElement::placeholderValue() const
{
return m_editControlOwner ? m_editControlOwner->formatDateTimeFieldsState(valueAsDateTimeFieldsState(DateTimePlaceholderIfNoValue::Yes)) : emptyString();
}

DateTimeFieldsState DateTimeEditElement::valueAsDateTimeFieldsState(DateTimePlaceholderIfNoValue placeholderIfNoValue) const
{
DateTimeFieldsState dateTimeFieldsState;
for (auto& field : m_fields)
field->populateDateTimeFieldsState(dateTimeFieldsState);
field->populateDateTimeFieldsState(dateTimeFieldsState, placeholderIfNoValue);
return dateTimeFieldsState;
}

Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/html/shadow/DateTimeEditElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class DateTimeEditElement final : public HTMLDivElement, public DateTimeFieldEle
void setEmptyValue(const LayoutParameters&);
void setValueAsDate(const LayoutParameters&, const DateComponents&);
String value() const;
String placeholderValue() const;
bool editableFieldsHaveValues() const;

private:
Expand All @@ -92,7 +93,7 @@ class DateTimeEditElement final : public HTMLDivElement, public DateTimeFieldEle
size_t fieldIndexOf(const DateTimeFieldElement&) const;
DateTimeFieldElement* focusedFieldElement() const;
void layout(const LayoutParameters&);
DateTimeFieldsState valueAsDateTimeFieldsState() const;
DateTimeFieldsState valueAsDateTimeFieldsState(DateTimePlaceholderIfNoValue = DateTimePlaceholderIfNoValue::No) const;

bool focusOnNextFocusableField(size_t startIndex);

Expand Down
5 changes: 4 additions & 1 deletion Source/WebCore/html/shadow/DateTimeFieldElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ class RenderStyle;

struct DateTimeFieldsState;

enum class DateTimePlaceholderIfNoValue : bool { No, Yes };

class DateTimeFieldElement : public HTMLDivElement {
WTF_MAKE_ISO_ALLOCATED(DateTimeFieldElement);
public:
Expand All @@ -65,7 +67,7 @@ class DateTimeFieldElement : public HTMLDivElement {
String visibleValue() const;

virtual bool hasValue() const = 0;
virtual void populateDateTimeFieldsState(DateTimeFieldsState&) = 0;
virtual void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue = DateTimePlaceholderIfNoValue::No) = 0;
virtual void setEmptyValue(EventBehavior = DispatchNoEvent) = 0;
virtual void setValueAsDate(const DateComponents&) = 0;
virtual void setValueAsInteger(int, EventBehavior = DispatchNoEvent) = 0;
Expand All @@ -81,6 +83,7 @@ class DateTimeFieldElement : public HTMLDivElement {
void updateVisibleValue(EventBehavior);
virtual void adjustMinInlineSize(RenderStyle&) const = 0;
virtual int valueAsInteger() const = 0;
virtual int placeholderValueAsInteger() const = 0;
virtual void handleKeyboardEvent(KeyboardEvent&) = 0;
virtual void handleBlurEvent(Event&);

Expand Down
38 changes: 27 additions & 11 deletions Source/WebCore/html/shadow/DateTimeFieldElements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ Ref<DateTimeDayFieldElement> DateTimeDayFieldElement::create(Document& document,
return element;
}

void DateTimeDayFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state)
void DateTimeDayFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state, DateTimePlaceholderIfNoValue placeholderIfNoValue)
{
if (hasValue())
state.dayOfMonth = valueAsInteger();
else if (placeholderIfNoValue == DateTimePlaceholderIfNoValue::Yes)
state.dayOfMonth = placeholderValueAsInteger();
}

void DateTimeDayFieldElement::setValueAsDate(const DateComponents& date)
Expand All @@ -84,12 +86,12 @@ Ref<DateTimeHourFieldElement> DateTimeHourFieldElement::create(Document& documen
return element;
}

void DateTimeHourFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state)
void DateTimeHourFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state, DateTimePlaceholderIfNoValue placeholderIfNoValue)
{
if (!hasValue())
if (!hasValue() && placeholderIfNoValue == DateTimePlaceholderIfNoValue::No)
return;

int value = valueAsInteger();
int value = hasValue() ? valueAsInteger() : placeholderValueAsInteger();

switch (maximum()) {
case 11:
Expand Down Expand Up @@ -157,10 +159,12 @@ void DateTimeMeridiemFieldElement::updateAriaValueAttributes()
setAttributeWithoutSynchronization(HTMLNames::aria_valuetextAttr, AtomString { visibleValue() });
}

void DateTimeMeridiemFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state)
void DateTimeMeridiemFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state, DateTimePlaceholderIfNoValue placeholderIfNoValue)
{
if (hasValue())
state.meridiem = valueAsInteger() ? DateTimeFieldsState::Meridiem::PM : DateTimeFieldsState::Meridiem::AM;
else if (placeholderIfNoValue == DateTimePlaceholderIfNoValue::Yes)
state.meridiem = placeholderValueAsInteger() ? DateTimeFieldsState::Meridiem::PM : DateTimeFieldsState::Meridiem::AM;
}

void DateTimeMeridiemFieldElement::setValueAsDate(const DateComponents& date)
Expand Down Expand Up @@ -198,10 +202,12 @@ Ref<DateTimeMillisecondFieldElement> DateTimeMillisecondFieldElement::create(Doc
return element;
}

void DateTimeMillisecondFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state)
void DateTimeMillisecondFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state, DateTimePlaceholderIfNoValue placeholderIfNoValue)
{
if (hasValue())
state.millisecond = valueAsInteger();
else if (placeholderIfNoValue == DateTimePlaceholderIfNoValue::Yes)
state.millisecond = placeholderValueAsInteger();
}

void DateTimeMillisecondFieldElement::setValueAsDate(const DateComponents& date)
Expand All @@ -226,10 +232,12 @@ Ref<DateTimeMinuteFieldElement> DateTimeMinuteFieldElement::create(Document& doc
return element;
}

void DateTimeMinuteFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state)
void DateTimeMinuteFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state, DateTimePlaceholderIfNoValue placeholderIfNoValue)
{
if (hasValue())
state.minute = valueAsInteger();
else if (placeholderIfNoValue == DateTimePlaceholderIfNoValue::Yes)
state.minute = placeholderValueAsInteger();
}

void DateTimeMinuteFieldElement::setValueAsDate(const DateComponents& date)
Expand All @@ -254,10 +262,12 @@ Ref<DateTimeMonthFieldElement> DateTimeMonthFieldElement::create(Document& docum
return element;
}

void DateTimeMonthFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state)
void DateTimeMonthFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state, DateTimePlaceholderIfNoValue placeholderIfNoValue)
{
if (hasValue())
state.month = valueAsInteger();
else if (placeholderIfNoValue == DateTimePlaceholderIfNoValue::Yes)
state.month = placeholderValueAsInteger();
}

void DateTimeMonthFieldElement::setValueAsDate(const DateComponents& date)
Expand All @@ -283,10 +293,12 @@ Ref<DateTimeSecondFieldElement> DateTimeSecondFieldElement::create(Document& doc
return element;
}

void DateTimeSecondFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state)
void DateTimeSecondFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state, DateTimePlaceholderIfNoValue placeholderIfNoValue)
{
if (hasValue())
state.second = valueAsInteger();
else if (placeholderIfNoValue == DateTimePlaceholderIfNoValue::Yes)
state.second = placeholderValueAsInteger();
}

void DateTimeSecondFieldElement::setValueAsDate(const DateComponents& date)
Expand All @@ -311,10 +323,12 @@ Ref<DateTimeSymbolicMonthFieldElement> DateTimeSymbolicMonthFieldElement::create
return element;
}

void DateTimeSymbolicMonthFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state)
void DateTimeSymbolicMonthFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state, DateTimePlaceholderIfNoValue placeholderIfNoValue)
{
if (hasValue())
state.month = valueAsInteger() + 1;
else if (placeholderIfNoValue == DateTimePlaceholderIfNoValue::Yes)
state.month = placeholderValueAsInteger() + 1;
}

void DateTimeSymbolicMonthFieldElement::setValueAsDate(const DateComponents& date)
Expand All @@ -339,10 +353,12 @@ Ref<DateTimeYearFieldElement> DateTimeYearFieldElement::create(Document& documen
return element;
}

void DateTimeYearFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state)
void DateTimeYearFieldElement::populateDateTimeFieldsState(DateTimeFieldsState& state, DateTimePlaceholderIfNoValue placeholderIfNoValue)
{
if (hasValue())
state.year = valueAsInteger();
else if (placeholderIfNoValue == DateTimePlaceholderIfNoValue::Yes)
state.year = placeholderValueAsInteger();
}

void DateTimeYearFieldElement::setValueAsDate(const DateComponents& date)
Expand Down
18 changes: 9 additions & 9 deletions Source/WebCore/html/shadow/DateTimeFieldElements.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class DateTimeDayFieldElement final : public DateTimeNumericFieldElement {

// DateTimeFieldElement functions:
void setValueAsDate(const DateComponents&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue) final;
};

class DateTimeHourFieldElement final : public DateTimeNumericFieldElement {
Expand All @@ -58,7 +58,7 @@ class DateTimeHourFieldElement final : public DateTimeNumericFieldElement {

// DateTimeFieldElement functions:
void setValueAsDate(const DateComponents&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue) final;
};

class DateTimeMeridiemFieldElement final : public DateTimeSymbolicFieldElement {
Expand All @@ -76,7 +76,7 @@ class DateTimeMeridiemFieldElement final : public DateTimeSymbolicFieldElement {
void setValueAsDate(const DateComponents&) final;
void setValueAsInteger(int, EventBehavior = DispatchNoEvent) final;

void populateDateTimeFieldsState(DateTimeFieldsState&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue) final;
};

class DateTimeMillisecondFieldElement final : public DateTimeNumericFieldElement {
Expand All @@ -90,7 +90,7 @@ class DateTimeMillisecondFieldElement final : public DateTimeNumericFieldElement

// DateTimeFieldElement functions:
void setValueAsDate(const DateComponents&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue) final;
};

class DateTimeMinuteFieldElement final : public DateTimeNumericFieldElement {
Expand All @@ -104,7 +104,7 @@ class DateTimeMinuteFieldElement final : public DateTimeNumericFieldElement {

// DateTimeFieldElement functions:
void setValueAsDate(const DateComponents&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue) final;
};

class DateTimeMonthFieldElement final : public DateTimeNumericFieldElement {
Expand All @@ -118,7 +118,7 @@ class DateTimeMonthFieldElement final : public DateTimeNumericFieldElement {

// DateTimeFieldElement functions:
void setValueAsDate(const DateComponents&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue) final;
};

class DateTimeSecondFieldElement final : public DateTimeNumericFieldElement {
Expand All @@ -132,7 +132,7 @@ class DateTimeSecondFieldElement final : public DateTimeNumericFieldElement {

// DateTimeFieldElement functions:
void setValueAsDate(const DateComponents&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue) final;
};

class DateTimeSymbolicMonthFieldElement final : public DateTimeSymbolicFieldElement {
Expand All @@ -146,7 +146,7 @@ class DateTimeSymbolicMonthFieldElement final : public DateTimeSymbolicFieldElem

// DateTimeFieldElement functions:
void setValueAsDate(const DateComponents&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue) final;
};

class DateTimeYearFieldElement final : public DateTimeNumericFieldElement {
Expand All @@ -160,7 +160,7 @@ class DateTimeYearFieldElement final : public DateTimeNumericFieldElement {

// DateTimeFieldElement functions:
void setValueAsDate(const DateComponents&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&) final;
void populateDateTimeFieldsState(DateTimeFieldsState&, DateTimePlaceholderIfNoValue) final;
};

} // namespace WebCore
Expand Down
6 changes: 1 addition & 5 deletions Source/WebCore/html/shadow/DateTimeNumericFieldElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ DateTimeNumericFieldElement::DateTimeNumericFieldElement(Document& document, Fie
: DateTimeFieldElement(document, fieldOwner)
, m_range(range)
, m_placeholder(formatValue(placeholder))
, m_placeholderValue(placeholder)
{
}

Expand Down Expand Up @@ -162,11 +163,6 @@ String DateTimeNumericFieldElement::placeholderValue() const
return m_placeholder;
}

int DateTimeNumericFieldElement::valueAsInteger() const
{
return m_hasValue ? m_value : -1;
}

void DateTimeNumericFieldElement::handleKeyboardEvent(KeyboardEvent& keyboardEvent)
{
if (keyboardEvent.type() != eventNames().keypressEvent)
Expand Down
Loading

0 comments on commit f2989cc

Please sign in to comment.