From d400a98b87e7b0405be8a01a0f526b7449757b7b Mon Sep 17 00:00:00 2001
From: Jessica Jong <jjong@mozilla.com>
Date: Thu, 29 Jun 2017 11:47:00 -0400
Subject: [PATCH] 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
---
 dom/html/HTMLInputElement.cpp                 |   2 +-
 dom/html/HTMLInputElement.h                   |   7 +
 dom/html/input/DateTimeInputTypes.cpp         |  11 ++
 dom/html/input/DateTimeInputTypes.h           |   2 +
 dom/html/nsIDateTimeInputArea.idl             |   6 +
 dom/html/test/forms/mochitest.ini             |   2 +
 .../test_input_time_sec_millisec_field.html   | 134 +++++++++++++++++
 dom/webidl/HTMLInputElement.webidl            |   8 +
 layout/forms/nsDateTimeControlFrame.cpp       |  12 +-
 layout/forms/nsDateTimeControlFrame.h         |   3 +-
 toolkit/content/widgets/datetimebox.xml       | 139 +++++++++++++-----
 11 files changed, 288 insertions(+), 38 deletions(-)
 create mode 100644 dom/html/test/forms/test_input_time_sec_millisec_field.html

diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp
index 5c744fce7744..71fb287aa445 100644
--- 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) {
diff --git a/dom/html/HTMLInputElement.h b/dom/html/HTMLInputElement.h
index ac702d487e24..b51b52b3ba7a 100644
--- 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();
diff --git a/dom/html/input/DateTimeInputTypes.cpp b/dom/html/input/DateTimeInputTypes.cpp
index ac209fb74e1b..ca9c6e3c53de 100644
--- 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,
diff --git a/dom/html/input/DateTimeInputTypes.h b/dom/html/input/DateTimeInputTypes.h
index b95dad042524..311efbd032fd 100644
--- 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)
diff --git a/dom/html/nsIDateTimeInputArea.idl b/dom/html/nsIDateTimeInputArea.idl
index 67600845df79..b2a8627fbe34 100644
--- 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.
    */
diff --git a/dom/html/test/forms/mochitest.ini b/dom/html/test/forms/mochitest.ini
index c038fb03cf88..45d775b9263a 100644
--- 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]
diff --git a/dom/html/test/forms/test_input_time_sec_millisec_field.html b/dom/html/test/forms/test_input_time_sec_millisec_field.html
new file mode 100644
index 000000000000..f3a6aa3b34b0
--- /dev/null
+++ 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>
diff --git a/dom/webidl/HTMLInputElement.webidl b/dom/webidl/HTMLInputElement.webidl
index 594403316a76..fdbfe58e9651 100644
--- 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 {
diff --git a/layout/forms/nsDateTimeControlFrame.cpp b/layout/forms/nsDateTimeControlFrame.cpp
index 58f661b5c734..11b2995d64f9 100644
--- 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)
 {
diff --git a/layout/forms/nsDateTimeControlFrame.h b/layout/forms/nsDateTimeControlFrame.h
index 20f65aca2d69..0580f77794f2 100644
--- 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();
diff --git a/toolkit/content/widgets/datetimebox.xml b/toolkit/content/widgets/datetimebox.xml
index eccc8fa7e966..1faf1c4b3855 100644
--- 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>