Closed Bug 323829 Opened 15 years ago Closed 15 years ago

datepicker input field not readonly, focus issue

Categories

(Core Graveyard :: XForms, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aaronr, Assigned: doronr)

References

Details

(Keywords: fixed1.8.0.4, fixed1.8.1)

Attachments

(2 files)

I found a couple more datepicker bugs.  If you bind a datepicker as readonly, you can still update the value with the dropdown.  I'd venture to say that we probably should just show a readonly default input for datepickers that are read only and only allow the dropdown when it isn't readonly.

Also, there is a bug with focus.  If you have a datepicker and click on the button to get a drop down and put the focus on the left/right arrow buttons at the top of the calendar, then there is no way to get the focus back off of those buttons using only the keyboard without dismissing the dropdown.
Attached file testcase
Attached patch patchSplinter Review
Attachment #208884 - Flags: review?(aaronr)
Comment on attachment 208884 [details] [diff] [review]
patch

>Index: extensions/xforms/resources/content/xforms.xml
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/xforms.xml,v
>retrieving revision 1.21
>diff -u -r1.21 xforms.xml
>--- extensions/xforms/resources/content/xforms.xml	29 Nov 2005 22:08:59 -0000	1.21
>+++ extensions/xforms/resources/content/xforms.xml	18 Jan 2006 18:20:00 -0000
>@@ -149,7 +149,7 @@
>       </method>
>     </implementation>
>   </binding>
>-  
>+
>   <!-- OUTPUT: <DEFAULT> -->
>   <binding id="xformswidget-output"
>            extends="chrome://xforms/content/xforms.xml#xformswidget-base">
>@@ -158,7 +158,7 @@
>       <html:span class="xf-value" anonid="content"></html:span>
>       <children/>
>     </content>
>-    
>+
>     <implementation implements="nsIXFormsUIWidget">
>       <method name="refresh">
>         <body>
>@@ -408,6 +408,14 @@
>     <implementation>
>       <method name="refresh">
>         <body>
>+          this.inputField.value = this.stringValue;
>+          if (this.accessors.isReadonly()) {
>+            this.inputField.setAttribute("readonly", "readonly");
>+            this.dropmarker.setAttribute("disabled", "true");
>+          } else {
>+            this.inputField.removeAttribute("readonly");
>+            this.dropmarker.removeAttribute("disabled");
>+          }
>           return true;
>         </body>
>       </method>
>@@ -423,6 +431,17 @@
>         </getter>
>       </property>
> 
>+      <field name="_dropmarker">null</field>
>+      <property name="dropmarker" readonly="true">
>+        <getter>
>+          if (!this._dropmarker) {
>+            this._dropmarker =
>+              document.getAnonymousElementByAttribute(this, "anonid", "dropmarker");
>+          }
>+          return this._dropmarker;
>+        </getter>
>+      </property>
>+
>       <field name="_dateField">null</field>
> 
>       <property name="dateField" readonly="true">
>@@ -457,13 +476,13 @@
>       <method name="_showPicker">
>         <body>
>         <![CDATA[
>-          // show the picker
>-          var picker = this.picker;
>-
>-          if (this._isPickerVisible) {
>+          if (this._isPickerVisible || this.accessors.isReadonly()) {
>             return;
>           }
> 
>+          // show the picker
>+          var picker = this.picker;
>+
>           picker.style.display = "block";
>           this._isPickerVisible = true;
> 
>@@ -730,15 +749,19 @@
>         <parameter name="aCellNum"/>
>         <body>
>         <![CDATA[
>-          if (aCellNum == this._currentCellIndex)
>-            return;
>-
>-          this._cells[this._currentCellIndex].node.setAttribute("tabindex", "-1");
>+          if (aCellNum == this._currentCellIndex) {
>+            // make sure it is focused.  We keep the currentCellIndex even if we
>+            // switch to the prev/next buttons, so we just need to refocus the
>+            // cell.
>+            this._cells[this._currentCellIndex].node.focus();
>+          } else {
>+            this._cells[this._currentCellIndex].node.setAttribute("tabindex", "-1");
> 
>-          this._currentCellIndex = aCellNum;
>+            this._currentCellIndex = aCellNum;
> 
>-          this._cells[this._currentCellIndex].node.setAttribute("tabindex", "0");
>-          this._cells[this._currentCellIndex].node.focus();
>+            this._cells[this._currentCellIndex].node.setAttribute("tabindex", "0");
>+            this._cells[this._currentCellIndex].node.focus();
>+          }
>         ]]>
>         </body>
>       </method>
>@@ -783,7 +806,7 @@
>             if (currentElement.localName == "input") {
>               // if we are on the button, down should focus the current selected
>               // cell
>-              this.selectCell(this._currentCellIndex);
>+              this.selectCell(index);
>             } else if ((index + 7) < this._cells.length) {
>               this.selectCell(index + 7);
>             }
Attachment #208884 - Flags: review?(aaronr) → review+
Attachment #208884 - Flags: superreview?(smaug)
Attachment #208884 - Flags: superreview?(smaug) → review?(smaug)
Attachment #208884 - Flags: review?(smaug) → review+
Attachment #208884 - Flags: review+ → review?
Attachment #208884 - Flags: review? → review+
Blocks: 326372
Blocks: 326373
Checked in to trunk
Hardware: PC → All
Whiteboard: xf-to-branch
Status: NEW → ASSIGNED
Blocks: 326556
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Blocks: 332853
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.