Bug 1301310 - Hide input date/time picker only when input element blurs. r?mossop draft
authorJessica Jong <jjong@mozilla.com>
Thu, 20 Apr 2017 15:04:10 +0800
changeset 566960 6a58d7169684bc6892137702570bcc09812b7760
parent 566789 8e969cc9aff49f845678cba5b35d9dd8aa340f16
child 625476 0baefe85f0d82c8c886f1ea2b5fd4adf49fae195
push id55392
push userjjong@mozilla.com
push dateMon, 24 Apr 2017 07:46:15 +0000
reviewersmossop
bugs1301310
milestone55.0a1
Bug 1301310 - Hide input date/time picker only when input element blurs. r?mossop If we rely on XUL panel's default behavior, the picker is hidden and opened again when we jump from one inner field to another with a mouse click, this is because XUL pannel gets hidden when user clicks outside it with noautohide=false. In order to avoid this, we should close it explicitly only when input element blurs. MozReview-Commit-ID: GxPxd0wPWgM
browser/base/content/browser.xul
toolkit/content/widgets/datetimebox.xml
toolkit/content/widgets/datetimepopup.xml
toolkit/modules/DateTimePickerHelper.jsm
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -160,16 +160,17 @@
            level="parent"
            overflowpadding="30" />
 
     <panel id="DateTimePickerPanel"
            type="arrow"
            hidden="true"
            orient="vertical"
            noautofocus="true"
+           noautohide="true"
            consumeoutsideclicks="false"
            level="parent"
            tabspecific="true">
       <iframe id="dateTimePopupFrame"/>
     </panel>
 
     <!-- for select dropdowns. The menupopup is what shows the list of options,
          and the popuponly menulist makes things like the menuactive attributes
--- a/toolkit/content/widgets/datetimebox.xml
+++ b/toolkit/content/widgets/datetimebox.xml
@@ -1129,16 +1129,19 @@
         this.EVENTS.forEach((eventName) => {
           this.addEventListener(eventName, this, { mozSystemGroup: true });
         });
         // Handle keypress separately since we need to catch it on capturing.
         this.addEventListener("keypress", this, {
           capture: true,
           mozSystemGroup: true
         });
+        // This is to close the picker when input element blurs.
+        this.mInputElement.addEventListener("blur", this,
+                                            { mozSystemGroup: true });
       ]]>
       </constructor>
 
       <destructor>
       <![CDATA[
         this.mInputElement = null;
 
         this.EVENTS.forEach((eventName) => {
@@ -1584,42 +1587,50 @@
         ]]>
         </body>
       </method>
 
       <method name="onBlur">
         <parameter name="aEvent"/>
         <body>
         <![CDATA[
-          this.log("onBlur originalTarget: " + aEvent.originalTarget);
+          this.log("onBlur originalTarget: " + aEvent.originalTarget +
+            " target: " + aEvent.target);
 
           if (document.activeElement == this.mInputElement) {
             return;
           }
 
+          if (aEvent.target == this.mInputElement && this.mIsPickerOpen) {
+            this.mInputElement.closeDateTimePicker();
+          }
+
           let target = aEvent.originalTarget;
           target.setAttribute("typeBuffer", "");
           this.setInputValueFromFields();
           this.mInputElement.setFocusState(false);
         ]]>
         </body>
       </method>
 
       <method name="onKeyPress">
         <parameter name="aEvent"/>
         <body>
         <![CDATA[
           this.log("onKeyPress key: " + aEvent.key);
 
           switch (aEvent.key) {
-            // Close picker on Enter or Space key.
+            // Close picker on Enter, Escape or Space key.
             case "Enter":
+            case "Escape":
             case " ": {
-              this.mInputElement.closeDateTimePicker();
-              aEvent.preventDefault();
+              if (this.mIsPickerOpen) {
+                this.mInputElement.closeDateTimePicker();
+                aEvent.preventDefault();
+              }
               break;
             }
             case "Backspace": {
               let targetField = aEvent.originalTarget;
               this.clearFieldValue(targetField);
               this.setInputValueFromFields();
               aEvent.preventDefault();
               break;
@@ -1665,17 +1676,17 @@
           // a HTMLDivElement and when clicking on the reset button, it's a
           // HTMLButtonElement.
           if (aEvent.defaultPrevented || this.isDisabled() || this.isReadonly()) {
             return;
           }
 
           if (aEvent.originalTarget == this.mResetButton) {
             this.clearInputFields(false);
-          } else {
+          } else if (!this.mIsPickerOpen) {
             this.mInputElement.openDateTimePicker(this.getCurrentValue());
           }
         ]]>
         </body>
       </method>
     </implementation>
   </binding>
 
--- a/toolkit/content/widgets/datetimepopup.xml
+++ b/toolkit/content/widgets/datetimepopup.xml
@@ -23,21 +23,21 @@
       <field name="DATE_PICKER_WIDTH" readonly="true">"23.1em"</field>
       <field name="DATE_PICKER_HEIGHT" readonly="true">"20.7em"</field>
       <constructor><![CDATA[
         this.mozIntl = Components.classes["@mozilla.org/mozintl;1"]
                                  .getService(Components.interfaces.mozIMozIntl);
         // Notify DateTimePickerHelper.jsm that binding is ready.
         this.dispatchEvent(new CustomEvent("DateTimePickerBindingReady"));
       ]]></constructor>
-      <method name="loadPicker">
+      <method name="openPicker">
         <parameter name="type"/>
+        <parameter name="anchor"/>
         <parameter name="detail"/>
         <body><![CDATA[
-          this.hidden = false;
           this.type = type;
           this.pickerState = {};
           // TODO: Resize picker according to content zoom level
           this.style.fontSize = "10px";
           switch (type) {
             case "time": {
               this.detail = detail;
               this.dateTimePopupFrame.addEventListener("load", this, true);
@@ -50,27 +50,30 @@
               this.detail = detail;
               this.dateTimePopupFrame.addEventListener("load", this, true);
               this.dateTimePopupFrame.setAttribute("src", "chrome://global/content/datepicker.xhtml");
               this.dateTimePopupFrame.style.width = this.DATE_PICKER_WIDTH;
               this.dateTimePopupFrame.style.height = this.DATE_PICKER_HEIGHT;
               break;
             }
           }
+          this.hidden = false;
+          this.openPopup(anchor, "after_start", 0, 0);
         ]]></body>
       </method>
       <method name="closePicker">
         <body><![CDATA[
-          this.hidden = true;
           this.setInputBoxValue(true);
           this.pickerState = {};
           this.type = undefined;
           this.dateTimePopupFrame.removeEventListener("load", this, true);
           this.dateTimePopupFrame.contentDocument.removeEventListener("message", this);
           this.dateTimePopupFrame.setAttribute("src", "");
+          this.hidePopup();
+          this.hidden = true;
         ]]></body>
       </method>
       <method name="setPopupValue">
         <parameter name="data"/>
         <body><![CDATA[
           switch (this.type) {
             case "time": {
               this.postMessageToPicker({
@@ -305,17 +308,10 @@
         <body><![CDATA[
           if (this.dateTimePopupFrame.contentDocument.nodePrincipal.isSystemPrincipal) {
             this.dateTimePopupFrame.contentWindow.postMessage(data, "*");
           }
         ]]></body>
       </method>
 
     </implementation>
-    <handlers>
-      <handler event="popuphiding">
-        <![CDATA[
-          this.closePicker();
-        ]]>
-      </handler>
-    </handlers>
   </binding>
 </bindings>
--- a/toolkit/modules/DateTimePickerHelper.jsm
+++ b/toolkit/modules/DateTimePickerHelper.jsm
@@ -131,30 +131,29 @@ this.DateTimePickerHelper = {
 
     this.weakBrowser = Cu.getWeakReference(aBrowser);
     this.picker = aBrowser.dateTimePicker;
     if (!this.picker) {
       debug("aBrowser.dateTimePicker not found, exiting now.");
       return;
     }
     // The datetimepopup binding is only attached when it is needed.
-    // Check if loadPicker method is present to determine if binding has
+    // Check if openPicker method is present to determine if binding has
     // been attached. If not, attach the binding first before calling it.
-    if (!this.picker.loadPicker) {
+    if (!this.picker.openPicker) {
       let bindingPromise = new Promise(resolve => {
         this.picker.addEventListener("DateTimePickerBindingReady",
                                      resolve, {once: true});
       });
       this.picker.setAttribute("active", true);
       yield bindingPromise;
     }
-    this.picker.loadPicker(type, detail);
     // The arrow panel needs an anchor to work. The popupAnchor (this._anchor)
     // is a transparent div that the arrow can point to.
-    this.picker.openPopup(this._anchor, "after_start", 0, 0);
+    this.picker.openPicker(type, this._anchor, detail);
 
     this.addPickerListeners();
   }),
 
   // Picker is closed, do some cleanup.
   close() {
     this.removePickerListeners();
     this.picker = null;