Bug 1374967 - Part 2: Consider step when deciding whether to show second/millisecond field. r=smaug

We should consider step and step base when deciding whether to show second and millisecond field, since step and step base can affect the valid time intervals, and the valid intervals may have second/millisecond part. MozReview-Commit-ID: H4mJvLTvBOM
2017-06-29 11:47:00 -04:00 · 2017-06-29 11:47:00 -04:00 · d400a98b87
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@ -3109,7 +3109,7 @@ HTMLInputElement::SetValueInternal(const nsAString& aValue,
                   !(aFlags & nsTextEditorState::eSetValue_BySetUserInput)) {
          nsDateTimeControlFrame* frame = do_QueryFrame(GetPrimaryFrame());
          if (frame) {
-            frame->UpdateInputBoxValue();
+            frame->OnValueChanged();
          }
        }
        if (mDoneCreating) {
--- a/dom/html/HTMLInputElement.h
+++ b/dom/html/HTMLInputElement.h
@ -846,6 +846,13 @@ public:
   */
  void UpdateValidityState();

+  /*
+   * The following are called from datetime input box binding to get the
+   * corresponding computed values.
+   */
+  double GetStepAsDouble() { return GetStep().toDouble(); }
+  double GetStepBaseAsDouble() { return GetStepBase().toDouble(); }
+
  HTMLInputElement* GetOwnerNumberControl();

  void StartNumberControlSpinnerSpin();
--- a/dom/html/input/DateTimeInputTypes.cpp
+++ b/dom/html/input/DateTimeInputTypes.cpp
@ -102,6 +102,17 @@ DateTimeInputTypeBase::HasBadInput() const
  return frame->HasBadInput();;
 }

+nsresult
+DateTimeInputTypeBase::MinMaxStepAttrChanged()
+{
+  nsDateTimeControlFrame* frame = do_QueryFrame(GetPrimaryFrame());
+  if (frame) {
+    frame->OnMinMaxStepAttrChanged();
+  }
+
+  return NS_OK;
+}
+
 bool
 DateTimeInputTypeBase::GetTimeFromMs(double aValue, uint16_t* aHours,
                                     uint16_t* aMinutes, uint16_t* aSeconds,
--- a/dom/html/input/DateTimeInputTypes.h
+++ b/dom/html/input/DateTimeInputTypes.h
@ -20,6 +20,8 @@ public:
  bool HasStepMismatch(bool aUseZeroIfValueNaN) const override;
  bool HasBadInput() const override;

+  nsresult MinMaxStepAttrChanged() override;
+
 protected:
  explicit DateTimeInputTypeBase(mozilla::dom::HTMLInputElement* aInputElement)
    : InputType(aInputElement)
--- a/dom/html/nsIDateTimeInputArea.idl
+++ b/dom/html/nsIDateTimeInputArea.idl
@ -14,6 +14,12 @@ interface nsIDateTimeInputArea : nsISupports
   */
  void notifyInputElementValueChanged();

+  /**
+   * Called from DOM/Layout when input element min, max or step attribute has
+   * changed.
+   */
+  void notifyMinMaxStepAttrChanged();
+
  /**
   * Called when date/time picker value has changed.
   */
--- a/dom/html/test/forms/mochitest.ini
+++ b/dom/html/test/forms/mochitest.ini
@ -74,6 +74,8 @@ skip-if = os == "android"
 [test_input_textarea_set_value_no_scroll.html]
 [test_input_time_key_events.html]
 skip-if = os == "android"
+[test_input_time_sec_millisec_field.html]
+skip-if = os == "android"
 [test_input_types_pref.html]
 [test_input_typing_sanitization.html]
 [test_input_untrusted_key_events.html]
--- a/dom/html/test/forms/test_input_time_sec_millisec_field.html
+++ b/dom/html/test/forms/test_input_time_sec_millisec_field.html
@ -0,0 +1,134 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1374967
+-->
+<head>
+  <title>Test second and millisecond fields in input type=time</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <meta charset="UTF-8">
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1374967">Mozilla Bug 1374967</a>
+<p id="display"></p>
+<div id="content">
+  <input id="input1" type="time">
+  <input id="input2" type="time" value="12:30:40">
+  <input id="input3" type="time" value="12:30:40.567">
+  <input id="input4" type="time" step="1">
+  <input id="input5" type="time" step="61">
+  <input id="input6" type="time" step="120">
+  <input id="input7" type="time" step="0.01">
+  <input id="input8" type="time" step="0.001">
+  <input id="input9" type="time" step="1.001">
+  <input id="input10" type="time" min="01:30:05">
+  <input id="input11" type="time" min="01:30:05.100">
+  <input id="dummy">
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+SimpleTest.waitForExplicitFinish();
+SimpleTest.waitForFocus(function() {
+  test();
+  SimpleTest.finish();
+});
+
+const NUM_OF_FIELDS_DEFAULT = 3;
+const NUM_OF_FIELDS_WITH_SECOND = NUM_OF_FIELDS_DEFAULT + 1;
+const NUM_OF_FIELDS_WITH_MILLISEC = NUM_OF_FIELDS_WITH_SECOND + 1;
+
+function countNumberOfFields(aElement) {
+  is(aElement.type, "time", "Input element type should be 'time'");
+
+  let inputRect = aElement.getBoundingClientRect();
+  let firstField_X = 15;
+  let firstField_Y = inputRect.height / 2;
+
+  // Make sure to start on the first field.
+  synthesizeMouse(aElement, firstField_X, firstField_Y, {});
+  is(document.activeElement, aElement, "Input element should be focused");
+
+  let n = 0;
+  while (document.activeElement == aElement) {
+    n++;
+    synthesizeKey("VK_TAB", {});
+  }
+
+  return n;
+}
+
+function test() {
+  // Normal input time element.
+  let elem = document.getElementById("input1");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_DEFAULT, "Default input time");
+
+  // Dynamically changing the value with second part.
+  elem.value = "10:20:30";
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_SECOND,
+     "Input time after changing value with second part");
+
+  // Dynamically changing the step to 1 millisecond.
+  elem.step = "0.001";
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_MILLISEC,
+     "Input time after changing step to 1 millisecond");
+
+  // Input time with value with second part.
+  elem = document.getElementById("input2");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_SECOND,
+     "Input time with value with second part");
+
+  // Input time with value with second and millisecond part.
+  elem = document.getElementById("input3");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_MILLISEC,
+     "Input time with value with second and millisecond part");
+
+  // Input time with step set as 1 second.
+  elem = document.getElementById("input4");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_SECOND,
+     "Input time with step set as 1 second");
+
+  // Input time with step set as 61 seconds.
+  elem = document.getElementById("input5");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_SECOND,
+     "Input time with step set as 61 seconds");
+
+  // Input time with step set as 2 minutes.
+  elem = document.getElementById("input6");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_DEFAULT,
+     "Input time with step set as 2 minutes");
+
+  // Input time with step set as 10 milliseconds.
+  elem = document.getElementById("input7");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_MILLISEC,
+     "Input time with step set as 10 milliseconds");
+
+  // Input time with step set as 100 milliseconds.
+  elem = document.getElementById("input8");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_MILLISEC,
+     "Input time with step set as 100 milliseconds");
+
+  // Input time with step set as 1001 milliseconds.
+  elem = document.getElementById("input9");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_MILLISEC,
+     "Input time with step set as 1001 milliseconds");
+
+  // Input time with min with second part and default step (60 seconds). Note
+  // that step base is min, when there is a min.
+  elem = document.getElementById("input10");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_SECOND,
+     "Input time with min with second part");
+
+  // Input time with min with second and millisecond part and default step (60
+  // seconds). Note that step base is min, when there is a min.
+  elem = document.getElementById("input11");
+  is(countNumberOfFields(elem), NUM_OF_FIELDS_WITH_MILLISEC,
+     "Input time with min with second and millisecond part");
+}
+
+</script>
+</pre>
+</body>
+</html>
--- a/dom/webidl/HTMLInputElement.webidl
+++ b/dom/webidl/HTMLInputElement.webidl
@ -261,6 +261,14 @@ partial interface HTMLInputElement {

  [Pref="dom.forms.datetime", Func="IsChromeOrXBL"]
  void updateValidityState();
+
+  [Pref="dom.forms.datetime", Func="IsChromeOrXBL",
+   BinaryName="getStepAsDouble"]
+  double getStep();
+
+  [Pref="dom.forms.datetime", Func="IsChromeOrXBL",
+   BinaryName="getStepBaseAsDouble"]
+  double getStepBase();
 };

 partial interface HTMLInputElement {
--- a/layout/forms/nsDateTimeControlFrame.cpp
+++ b/layout/forms/nsDateTimeControlFrame.cpp
@ -55,7 +55,7 @@ nsDateTimeControlFrame::DestroyFrom(nsIFrame* aDestructRoot)
 }

 void
-nsDateTimeControlFrame::UpdateInputBoxValue()
+nsDateTimeControlFrame::OnValueChanged()
 {
  nsCOMPtr<nsIDateTimeInputArea> inputAreaContent =
    do_QueryInterface(mInputAreaContent);
@ -64,6 +64,16 @@ nsDateTimeControlFrame::UpdateInputBoxValue()
  }
 }

+void
+nsDateTimeControlFrame::OnMinMaxStepAttrChanged()
+{
+  nsCOMPtr<nsIDateTimeInputArea> inputAreaContent =
+    do_QueryInterface(mInputAreaContent);
+  if (inputAreaContent) {
+    inputAreaContent->NotifyMinMaxStepAttrChanged();
+  }
+}
+
 void
 nsDateTimeControlFrame::SetValueFromPicker(const DateTimeValue& aValue)
 {
--- a/layout/forms/nsDateTimeControlFrame.h
+++ b/layout/forms/nsDateTimeControlFrame.h
@ -73,7 +73,8 @@ public:
  nsresult AttributeChanged(int32_t aNameSpaceID, nsIAtom* aAttribute,
                            int32_t aModType) override;

-  void UpdateInputBoxValue();
+  void OnValueChanged();
+  void OnMinMaxStepAttrChanged();
  void SetValueFromPicker(const DateTimeValue& aValue);
  void HandleFocusEvent();
  void HandleBlurEvent();
--- a/toolkit/content/widgets/datetimebox.xml
+++ b/toolkit/content/widgets/datetimebox.xml
@ -431,6 +431,9 @@
    </resources>

    <implementation>
+      <property name="kMsPerSecond" readonly="true" onget="return 1000;" />
+      <property name="kMsPerMinute" readonly="true" onget="return (60 * 1000);" />
+
      <constructor>
      <![CDATA[
        const kDefaultAMString = "AM";
@ -526,22 +529,73 @@
        </body>
      </method>

-      <method name="reBuildEditFields">
+      <method name="shouldShowSecondField">
        <body>
-          <![CDATA[
-            let root =
-              document.getAnonymousElementByAttribute(this, "anonid", "edit-wrapper");
-            while (root.firstChild) {
-              root.firstChild.remove();
-            }
+        <![CDATA[
+          let { second } = this.getInputElementValues();
+          if (second != undefined) {
+            return true;
+          }

-            this.mHourField = null;
-            this.mMinuteField = null;
-            this.mSecondField = null;
-            this.mMillisecField = null;
+          let stepBase = this.mInputElement.getStepBase();
+          if ((stepBase % this.kMsPerMinute) != 0) {
+            return true;
+          }

-            this.buildEditFields();
-          ]]>
+          let step = this.mInputElement.getStep();
+          if ((step % this.kMsPerMinute) != 0) {
+            return true;
+          }
+
+          return false;
+        ]]>
+        </body>
+      </method>
+
+      <method name="shouldShowMillisecField">
+        <body>
+        <![CDATA[
+          let { millisecond } = this.getInputElementValues();
+          if (millisecond != undefined) {
+            return true;
+          }
+
+          let stepBase = this.mInputElement.getStepBase();
+          if ((stepBase % this.kMsPerSecond) != 0) {
+            return true;
+          }
+
+          let step = this.mInputElement.getStep();
+          if ((step % this.kMsPerSecond) != 0) {
+            return true;
+          }
+
+          return false;
+        ]]>
+        </body>
+      </method>
+
+      <method name="rebuildEditFieldsIfNeeded">
+        <body>
+        <![CDATA[
+          if ((this.shouldShowSecondField() == this.hasSecondField()) &&
+              (this.shouldShowMillisecField() == this.hasMillisecField())) {
+            return;
+          }
+
+          let root =
+            document.getAnonymousElementByAttribute(this, "anonid", "edit-wrapper");
+          while (root.firstChild) {
+            root.firstChild.remove();
+          }
+
+          this.mHourField = null;
+          this.mMinuteField = null;
+          this.mSecondField = null;
+          this.mMillisecField = null;
+
+          this.buildEditFields();
+        ]]>
        </body>
      </method>

@ -552,8 +606,6 @@
          let root =
            document.getAnonymousElementByAttribute(this, "anonid", "edit-wrapper");

-          let { second, millisecond } = this.getInputElementValues();
-
          let options = {
            hour: "numeric",
            minute: "numeric",
@ -572,18 +624,18 @@
              this.mDayPeriodPlaceHolder, false);
          }

-          if (second != undefined) {
+          if (this.shouldShowSecondField()) {
            options["second"] = "numeric";
            this.mSecondField = this.createEditField(this.mSecondPlaceHolder,
-              true, this.mMaxLength, this.mMaxLength, this.mMinSecond, this.mMaxSecond,
-              this.mMinSecPageUpDownInterval);
-          }
+              true, this.mMaxLength, this.mMaxLength, this.mMinSecond,
+              this.mMaxSecond, this.mMinSecPageUpDownInterval);

-          if (millisecond != undefined) {
-            this.mMillisecField = this.createEditField(this.mMillisecPlaceHolder,
-              true, this.mMillisecMaxLength, this.mMillisecMaxLength,
-              this.mMinMillisecond, this.mMaxMillisecond,
-              this.mMinSecPageUpDownInterval);
+            if (this.shouldShowMillisecField()) {
+              this.mMillisecField = this.createEditField(
+                this.mMillisecPlaceHolder, true, this.mMillisecMaxLength,
+                this.mMillisecMaxLength, this.mMinMillisecond,
+                this.mMaxMillisecond, this.mMinSecPageUpDownInterval);
+            }
          }

          let fragment = document.createDocumentFragment();
@ -598,7 +650,7 @@
                break;
              case "second":
                fragment.appendChild(this.mSecondField);
-                if (millisecond != undefined) {
+                if (this.shouldShowMillisecField()) {
                  // Intl.DateTimeFormat does not support millisecond, so we
                  // need to handle this on our own.
                  let span = document.createElementNS(HTML_NS, "span");
@ -674,13 +726,9 @@
            return;
          }

-          // Second and millsecond part is optional, rebuild edit fields if
+          // Second and millisecond part are optional, rebuild edit fields if
          // needed.
-          if ((this.isEmpty(second) == this.hasSecondField()) ||
-              (this.isEmpty(millisecond) == this.hasMillisecField())) {
-            this.log("Edit fields need to be rebuilt.")
-            this.reBuildEditFields();
-          }
+          this.rebuildEditFieldsIfNeeded();

          this.setFieldValue(this.mHourField, hour);
          this.setFieldValue(this.mMinuteField, minute);
@ -689,11 +737,14 @@
                                                         : this.mAMIndicator);
          }

-          if (!this.isEmpty(second)) {
-            this.setFieldValue(this.mSecondField, second);
+          if (this.hasSecondField()) {
+            this.setFieldValue(this.mSecondField,
+              (second != undefined) ? second : 0);
          }
-          if (!this.isEmpty(millisecond)) {
-            this.setFieldValue(this.mMillisecField, millisecond);
+
+          if (this.hasMillisecField()) {
+            this.setFieldValue(this.mMillisecField,
+              (millisecond != undefined) ? millisecond : 0);
          }

          this.notifyPicker();
@ -832,6 +883,18 @@
        </body>
      </method>

+      <method name="notifyMinMaxStepAttrChanged">
+        <body>
+        <![CDATA[
+          // Second and millisecond part are optional, rebuild edit fields if
+          // needed.
+          this.rebuildEditFieldsIfNeeded();
+          // Fill in values again.
+          this.setFieldsFromInputValue();
+        ]]>
+        </body>
+      </method>
+
      <method name="incrementFieldValue">
        <parameter name="aTargetField"/>
        <parameter name="aTimes"/>
@ -1335,6 +1398,12 @@
        </body>
      </method>

+      <method name="notifyMinMaxStepAttrChanged">
+        <body>
+        <!-- No operation by default -->
+        </body>
+      </method>
+
      <method name="setValueFromPicker">
        <parameter name="aValue"/>
        <body>