Changeset 272368 in webkit


Ignore:
Timestamp:
Feb 4, 2021 6:59:24 AM (18 months ago)
Author:
Aditya Keerthi
Message:

[macOS] Selecting a date on datetime-local inputs unexpectedly adds second and millisecond fields
​https://bugs.webkit.org/show_bug.cgi?id=221350
<rdar://problem/73943517>

Reviewed by Devin Rousso.

Source/WebCore:

Currently, when setting the value of a datetime-local input using the
picker, the length of the current value of the input is used to determine
whether or not to return a value with second/millisecond precision.

This is approach is incorrect, since the value could be empty, while the
step attribute can specify second/millisecond precision. To fix, ensure
the DateTimeChooserParameters knows whether the input has second and
millisecond fields. That information can then be used by the UIProcess
to return a correctly formatted value to the WebProcess.

Test: fast/forms/datetimelocal/datetimelocal-editable-components/datetimelocal-choose-value-from-picker.html

  • html/BaseDateAndTimeInputType.cpp:

(WebCore::BaseDateAndTimeInputType::handleDOMActivateEvent):
(WebCore::BaseDateAndTimeInputType::didChangeValueFromControl):
(WebCore::BaseDateAndTimeInputType::setupDateTimeChooserParameters):

Moved this method from HTMLInputElement to the input type, since it is
specific to date/time input types, and to leverage the existing
shouldHaveSecondField and shouldHaveMillisecondField methods when
building the DateTimeChooserParameters.

  • html/BaseDateAndTimeInputType.h:
  • html/HTMLInputElement.cpp:
  • html/HTMLInputElement.h:
  • platform/DateTimeChooserParameters.h:

Added hasSecondField and hasMillisecondField members, so that the UIProcess
knows whether or not to return a string that contains seconds/milliseconds.

(WebCore::DateTimeChooserParameters::encode const):
(WebCore::DateTimeChooserParameters::decode):

Source/WebKit:

  • UIProcess/mac/WebDateTimePickerMac.mm:

(-[WKDateTimePicker updatePicker:]):
(-[WKDateTimePicker dateFormatStringForType:]):

Do not use the length of the value to determine whether or seconds and
milliseconds should be present, since the value can be empty.

Instead, use the new information in DateTimeChooserParameters, matching
the visual appearance of the input.

Tools:

Added a method to UIScriptController to simulate selecting a date using
the presented date picker.

  • TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
  • TestRunnerShared/UIScriptContext/UIScriptController.h:

(WTR::UIScriptController::chooseDateTimePickerValue):

  • WebKitTestRunner/mac/UIScriptControllerMac.h:
  • WebKitTestRunner/mac/UIScriptControllerMac.mm:

(WTR::UIScriptControllerMac::chooseDateTimePickerValue):

LayoutTests:

Added a test to to verify that the presence of seconds and milliseconds
in the value of a datetime-local input after selecting a date using the
picker matches the configuration.

  • fast/forms/datetimelocal/datetimelocal-editable-components/datetimelocal-choose-value-from-picker-expected.txt: Added.
  • fast/forms/datetimelocal/datetimelocal-editable-components/datetimelocal-choose-value-from-picker.html: Added.
  • resources/ui-helper.js:

(window.UIHelper.chooseDateTimePickerValue):

Location:
trunk
Files:
2 added
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r272366 r272368  
     12021-02-04  Aditya Keerthi  <akeerthi@apple.com>
     2
     3        [macOS] Selecting a date on datetime-local inputs unexpectedly adds second and millisecond fields
     4        https://bugs.webkit.org/show_bug.cgi?id=221350
     5        <rdar://problem/73943517>
     6
     7        Reviewed by Devin Rousso.
     8
     9        Added a test to to verify that the presence of seconds and milliseconds
     10        in the value of a datetime-local input after selecting a date using the
     11        picker matches the configuration.
     12
     13        * fast/forms/datetimelocal/datetimelocal-editable-components/datetimelocal-choose-value-from-picker-expected.txt: Added.
     14        * fast/forms/datetimelocal/datetimelocal-editable-components/datetimelocal-choose-value-from-picker.html: Added.
     15        * resources/ui-helper.js:
     16        (window.UIHelper.chooseDateTimePickerValue):
     17
    1182021-02-04  Martin Robinson  <mrobinson@igalia.com>
    219

  • trunk/LayoutTests/resources/ui-helper.js

    r272334 r272368  
    939939                uiController.uiScriptComplete(uiController.dateTimePickerValue);
    940940            })()`, valueAsString => resolve(parseFloat(valueAsString)));
     941        });
     942    }
     943
     944    static chooseDateTimePickerValue()
     945    {
     946        return new Promise((resolve) => {
     947            testRunner.runUIScript(`
     948                uiController.chooseDateTimePickerValue();
     949                uiController.uiScriptComplete();
     950            `, resolve);
    941951        });
    942952    }

  • trunk/Source/WebCore/ChangeLog

    r272367 r272368  
     12021-02-04  Aditya Keerthi  <akeerthi@apple.com>
     2
     3        [macOS] Selecting a date on datetime-local inputs unexpectedly adds second and millisecond fields
     4        https://bugs.webkit.org/show_bug.cgi?id=221350
     5        <rdar://problem/73943517>
     6
     7        Reviewed by Devin Rousso.
     8
     9        Currently, when setting the value of a datetime-local input using the
     10        picker, the length of the current value of the input is used to determine
     11        whether or not to return a value with second/millisecond precision.
     12
     13        This is approach is incorrect, since the value could be empty, while the
     14        step attribute can specify second/millisecond precision. To fix, ensure
     15        the DateTimeChooserParameters knows whether the input has second and
     16        millisecond fields. That information can then be used by the UIProcess
     17        to return a correctly formatted value to the WebProcess.
     18
     19        Test: fast/forms/datetimelocal/datetimelocal-editable-components/datetimelocal-choose-value-from-picker.html
     20
     21        * html/BaseDateAndTimeInputType.cpp:
     22        (WebCore::BaseDateAndTimeInputType::handleDOMActivateEvent):
     23        (WebCore::BaseDateAndTimeInputType::didChangeValueFromControl):
     24        (WebCore::BaseDateAndTimeInputType::setupDateTimeChooserParameters):
     25
     26        Moved this method from HTMLInputElement to the input type, since it is
     27        specific to date/time input types, and to leverage the existing
     28        shouldHaveSecondField and shouldHaveMillisecondField methods when
     29        building the DateTimeChooserParameters.
     30
     31        * html/BaseDateAndTimeInputType.h:
     32        * html/HTMLInputElement.cpp:
     33        * html/HTMLInputElement.h:
     34        * platform/DateTimeChooserParameters.h:
     35
     36        Added hasSecondField and hasMillisecondField members, so that the UIProcess
     37        knows whether or not to return a string that contains seconds/milliseconds.
     38
     39        (WebCore::DateTimeChooserParameters::encode const):
     40        (WebCore::DateTimeChooserParameters::decode):
     41
    1422021-02-04  Commit Queue  <commit-queue@webkit.org>
    243

  • trunk/Source/WebCore/html/BaseDateAndTimeInputType.cpp

    r272097 r272368  
    4141#include "Decimal.h"
    4242#include "FocusController.h"
     43#include "HTMLDataListElement.h"
    4344#include "HTMLDivElement.h"
    4445#include "HTMLInputElement.h"
    4546#include "HTMLNames.h"
     47#include "HTMLOptionElement.h"
    4648#include "KeyboardEvent.h"
    4749#include "Page.h"
    … …  
    293295
    294296    DateTimeChooserParameters parameters;
    295     if (!element()->setupDateTimeChooserParameters(parameters))
     297    if (!setupDateTimeChooserParameters(parameters))
    296298        return;
    297299
    … …  
    461463
    462464    DateTimeChooserParameters parameters;
    463     if (!element()->setupDateTimeChooserParameters(parameters))
     465    if (!setupDateTimeChooserParameters(parameters))
    464466        return;
    465467
    … …  
    497499}
    498500
     501bool BaseDateAndTimeInputType::setupDateTimeChooserParameters(DateTimeChooserParameters& parameters)
     502{
     503    ASSERT(element());
     504
     505    auto& element = *this->element();
     506    auto& document = element.document();
     507
     508    if (!document.view())
     509        return false;
     510
     511    parameters.type = element.type();
     512    parameters.minimum = element.minimum();
     513    parameters.maximum = element.maximum();
     514    parameters.required = element.isRequired();
     515
     516    if (!document.settings().langAttributeAwareFormControlUIEnabled())
     517        parameters.locale = defaultLanguage();
     518    else {
     519        AtomString computedLocale = element.computeInheritedLanguage();
     520        parameters.locale = computedLocale.isEmpty() ? AtomString(defaultLanguage()) : computedLocale;
     521    }
     522
     523    auto stepRange = createStepRange(AnyStepHandling::Reject);
     524    if (stepRange.hasStep()) {
     525        parameters.step = stepRange.step().toDouble();
     526        parameters.stepBase = stepRange.stepBase().toDouble();
     527    } else {
     528        parameters.step = 1.0;
     529        parameters.stepBase = 0;
     530    }
     531
     532    if (RenderElement* renderer = element.renderer())
     533        parameters.anchorRectInRootView = document.view()->contentsToRootView(renderer->absoluteBoundingBoxRect());
     534    else
     535        parameters.anchorRectInRootView = IntRect();
     536    parameters.currentValue = element.value();
     537
     538    auto* computedStyle = element.computedStyle();
     539    parameters.isAnchorElementRTL = computedStyle->direction() == TextDirection::RTL;
     540    parameters.useDarkAppearance = document.useDarkAppearance(computedStyle);
     541
     542    auto date = parseToDateComponents(element.value()).valueOr(DateComponents());
     543    parameters.hasSecondField = shouldHaveSecondField(date);
     544    parameters.hasMillisecondField = shouldHaveMillisecondField(date);
     545
     546#if ENABLE(DATALIST_ELEMENT)
     547    if (auto dataList = element.dataList()) {
     548        for (auto& option : dataList->suggestions()) {
     549            auto label = option.label();
     550            auto value = option.value();
     551            if (!element.isValidValue(value))
     552                continue;
     553            parameters.suggestionValues.append(element.sanitizeValue(value));
     554            parameters.localizedSuggestionValues.append(element.localizeValue(value));
     555            parameters.suggestionLabels.append(value == label ? String() : label);
     556        }
     557    }
     558#endif
     559
     560    return true;
     561}
     562
    499563void BaseDateAndTimeInputType::closeDateTimeChooser()
    500564{

  • trunk/Source/WebCore/html/BaseDateAndTimeInputType.h

    r272097 r272368  
    4444
    4545class DateComponents;
     46
     47struct DateTimeChooserParameters;
    4648
    4749// A super class of date, datetime, datetime-local, month, time, and week types.
    … …  
    134136    void didEndChooser() final;
    135137
     138    bool setupDateTimeChooserParameters(DateTimeChooserParameters&);
    136139    void closeDateTimeChooser();
    137140

  • trunk/Source/WebCore/html/HTMLInputElement.cpp

    r272354 r272368  
    3939#include "DateComponents.h"
    4040#include "DateTimeChooser.h"
    41 #include "DateTimeChooserParameters.h"
    4241#include "Document.h"
    4342#include "Editor.h"
    … …  
    21032102}
    21042103
    2105 #if ENABLE(DATE_AND_TIME_INPUT_TYPES)
    2106 
    2107 bool HTMLInputElement::setupDateTimeChooserParameters(DateTimeChooserParameters& parameters)
    2108 {
    2109     if (!document().view())
    2110         return false;
    2111 
    2112     parameters.type = type();
    2113     parameters.minimum = minimum();
    2114     parameters.maximum = maximum();
    2115     parameters.required = isRequired();
    2116 
    2117     if (!document().settings().langAttributeAwareFormControlUIEnabled())
    2118         parameters.locale = defaultLanguage();
    2119     else {
    2120         AtomString computedLocale = computeInheritedLanguage();
    2121         parameters.locale = computedLocale.isEmpty() ? AtomString(defaultLanguage()) : computedLocale;
    2122     }
    2123 
    2124     auto stepRange = createStepRange(AnyStepHandling::Reject);
    2125     if (stepRange.hasStep()) {
    2126         parameters.step = stepRange.step().toDouble();
    2127         parameters.stepBase = stepRange.stepBase().toDouble();
    2128     } else {
    2129         parameters.step = 1.0;
    2130         parameters.stepBase = 0;
    2131     }
    2132 
    2133     if (RenderElement* renderer = this->renderer())
    2134         parameters.anchorRectInRootView = document().view()->contentsToRootView(renderer->absoluteBoundingBoxRect());
    2135     else
    2136         parameters.anchorRectInRootView = IntRect();
    2137     parameters.currentValue = value();
    2138 
    2139     auto* computedStyle = this->computedStyle();
    2140     parameters.isAnchorElementRTL = computedStyle->direction() == TextDirection::RTL;
    2141     parameters.useDarkAppearance = document().useDarkAppearance(computedStyle);
    2142 
    2143 #if ENABLE(DATALIST_ELEMENT)
    2144     if (auto dataList = this->dataList()) {
    2145         for (auto& option : dataList->suggestions()) {
    2146             auto label = option.label();
    2147             auto value = option.value();
    2148             if (!isValidValue(value))
    2149                 continue;
    2150             parameters.suggestionValues.append(sanitizeValue(value));
    2151             parameters.localizedSuggestionValues.append(localizeValue(value));
    2152             parameters.suggestionLabels.append(value == label ? String() : label);
    2153         }
    2154     }
    2155 #endif
    2156 
    2157     return true;
    2158 }
    2159 
    2160 #endif
    2161 
    21622104void HTMLInputElement::capsLockStateMayHaveChanged()
    21632105{

  • trunk/Source/WebCore/html/HTMLInputElement.h

    r269587 r272368  
    4343class StepRange;
    4444
    45 struct DateTimeChooserParameters;
    46 
    4745struct InputElementClickState {
    4846    bool stateful { false };
    … …  
    327325    HTMLImageLoader* imageLoader() { return m_imageLoader.get(); }
    328326    HTMLImageLoader& ensureImageLoader();
    329 
    330 #if ENABLE(DATE_AND_TIME_INPUT_TYPES)
    331     bool setupDateTimeChooserParameters(DateTimeChooserParameters&);
    332 #endif
    333327
    334328    void capsLockStateMayHaveChanged();

  • trunk/Source/WebCore/platform/DateTimeChooserParameters.h

    r267131 r272368  
    4444    Vector<String> localizedSuggestionValues;
    4545    Vector<String> suggestionLabels;
    46     double minimum;
    47     double maximum;
    48     double step;
    49     double stepBase;
    50     bool required;
    51     bool isAnchorElementRTL;
    52     bool useDarkAppearance;
     46    double minimum { 0 };
     47    double maximum { 0 };
     48    double step { 0 };
     49    double stepBase { 0 };
     50    bool required { false };
     51    bool isAnchorElementRTL { false };
     52    bool useDarkAppearance { false };
     53    bool hasSecondField { false };
     54    bool hasMillisecondField { false };
    5355
    5456    template<class Encoder> void encode(Encoder&) const;
    … …  
    7375    encoder << isAnchorElementRTL;
    7476    encoder << useDarkAppearance;
     77    encoder << hasSecondField;
     78    encoder << hasMillisecondField;
    7579}
    7680
    … …  
    134138        return WTF::nullopt;
    135139
    136     return {{ WTFMove(type), anchorRectInRootView, WTFMove(locale), WTFMove(currentValue), WTFMove(suggestionValues), WTFMove(localizedSuggestionValues), WTFMove(suggestionLabels), minimum, maximum, step, stepBase, required, isAnchorElementRTL, useDarkAppearance }};
     140    bool hasSecondField;
     141    if (!decoder.decode(hasSecondField))
     142        return WTF::nullopt;
     143
     144    bool hasMillisecondField;
     145    if (!decoder.decode(hasMillisecondField))
     146        return WTF::nullopt;
     147
     148    return {{ WTFMove(type), anchorRectInRootView, WTFMove(locale), WTFMove(currentValue), WTFMove(suggestionValues), WTFMove(localizedSuggestionValues), WTFMove(suggestionLabels), minimum, maximum, step, stepBase, required, isAnchorElementRTL, useDarkAppearance, hasSecondField, hasMillisecondField }};
    137149}
    138150

  • trunk/Source/WebKit/ChangeLog

    r272362 r272368  
     12021-02-04  Aditya Keerthi  <akeerthi@apple.com>
     2
     3        [macOS] Selecting a date on datetime-local inputs unexpectedly adds second and millisecond fields
     4        https://bugs.webkit.org/show_bug.cgi?id=221350
     5        <rdar://problem/73943517>
     6
     7        Reviewed by Devin Rousso.
     8
     9        * UIProcess/mac/WebDateTimePickerMac.mm:
     10        (-[WKDateTimePicker updatePicker:]):
     11        (-[WKDateTimePicker dateFormatStringForType:]):
     12
     13        Do not use the length of the value to determine whether or seconds and
     14        milliseconds should be present, since the value can be empty.
     15
     16        Instead, use the new information in DateTimeChooserParameters, matching
     17        the visual appearance of the input.
     18
    1192021-02-04  Lauro Moura  <lmoura@igalia.com>
    220

  • trunk/Source/WebKit/UIProcess/mac/WebDateTimePickerMac.mm

    r269540 r272368  
    241241    NSString *currentDateValueString = _params.currentValue;
    242242
    243     [_dateFormatter setDateFormat:[self dateFormatStringForType:_params.type value:currentDateValueString]];
     243    [_dateFormatter setDateFormat:[self dateFormatStringForType:_params.type]];
    244244
    245245    if (![currentDateValueString length])
    … …  
    277277}
    278278
    279 - (NSString *)dateFormatStringForType:(NSString *)type value:(NSString *)value
     279- (NSString *)dateFormatStringForType:(NSString *)type
    280280{
    281281    if ([type isEqualToString:@"datetime-local"]) {
    282         // Add two additional characters for the string delimiters in 'T'.
    283         NSUInteger valueLengthForFormat = value.length + 2;
    284         if (valueLengthForFormat == kDateTimeFormatString.length)
    285             return kDateTimeFormatString;
    286         if (valueLengthForFormat == kDateTimeWithSecondsFormatString.length)
     282        if (_params.hasMillisecondField)
     283            return kDateTimeWithMillisecondsFormatString;
     284        if (_params.hasSecondField)
    287285            return kDateTimeWithSecondsFormatString;
    288         return kDateTimeWithMillisecondsFormatString;
     286        return kDateTimeFormatString;
    289287    }
    290288

  • trunk/Tools/ChangeLog

    r272365 r272368  
     12021-02-04  Aditya Keerthi  <akeerthi@apple.com>
     2
     3        [macOS] Selecting a date on datetime-local inputs unexpectedly adds second and millisecond fields
     4        https://bugs.webkit.org/show_bug.cgi?id=221350
     5        <rdar://problem/73943517>
     6
     7        Reviewed by Devin Rousso.
     8
     9        Added a method to UIScriptController to simulate selecting a date using
     10        the presented date picker.
     11
     12        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
     13        * TestRunnerShared/UIScriptContext/UIScriptController.h:
     14        (WTR::UIScriptController::chooseDateTimePickerValue):
     15        * WebKitTestRunner/mac/UIScriptControllerMac.h:
     16        * WebKitTestRunner/mac/UIScriptControllerMac.mm:
     17        (WTR::UIScriptControllerMac::chooseDateTimePickerValue):
     18
    1192021-02-04  Eleni Maria Stea  <estea@igalia.com>
    220

  • trunk/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl

    r271786 r272368  
    230230    readonly attribute boolean isShowingDateTimePicker;
    231231    readonly attribute double dateTimePickerValue;
     232    undefined chooseDateTimePickerValue();
    232233
    233234    // <datalist>

  • trunk/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h

    r271786 r272368  
    224224    virtual bool isShowingDateTimePicker() const { notImplemented(); return false; }
    225225    virtual double dateTimePickerValue() const { notImplemented(); return 0; }
     226    virtual void chooseDateTimePickerValue() { notImplemented(); }
    226227    virtual bool isShowingDataListSuggestions() const { notImplemented(); return false; }
    227228    virtual JSObjectRef calendarType() const { notImplemented(); return nullptr; }

  • trunk/Tools/WebKitTestRunner/mac/UIScriptControllerMac.h

    r267761 r272368  
    4646    bool isShowingDateTimePicker() const override;
    4747    double dateTimePickerValue() const override;
     48    void chooseDateTimePickerValue() override;
    4849    bool isShowingDataListSuggestions() const override;
    4950    void activateDataListSuggestion(unsigned index, JSValueRef callback) override;

  • trunk/Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm

    r267761 r272368  
    120120}
    121121
     122void UIScriptControllerMac::chooseDateTimePickerValue()
     123{
     124    for (NSWindow *childWindow in webView().window.childWindows) {
     125        if ([childWindow isKindOfClass:NSClassFromString(@"WKDateTimePickerWindow")]) {
     126            for (NSView *subview in childWindow.contentView.subviews) {
     127                if ([subview isKindOfClass:[NSDatePicker class]]) {
     128                    NSDatePicker *datePicker = (NSDatePicker *)subview;
     129                    [datePicker.target performSelector:datePicker.action withObject:datePicker];
     130                    return;
     131                }
     132            }
     133        }
     134    }
     135}
     136
    122137bool UIScriptControllerMac::isShowingDataListSuggestions() const
    123138{
Note: See TracChangeset for help on using the changeset viewer.