Closed Bug 332197 Opened 15 years ago Closed 15 years ago

support @selection="open" for xf:select1[appearance='minimal'] for xul

Categories

(Core Graveyard :: XForms, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

Details

(Keywords: fixed1.8.0.5, fixed1.8.1)

Attachments

(2 files, 2 obsolete files)

It's need to support @selection="open" for xf:select1[appearance='minimal'] for xul context. The bug I want to fix after bug 323851 fixing.
Status: NEW → ASSIGNED
Attached patch patch (obsolete) — Splinter Review
The patch doesn't support free enrty items when value is element node. I don't know how can item corresponding such value should be displayed.
Attachment #218403 - Flags: review?(aaronr)
could you please attach a testcase so that I can play around with the patch?
Attached patch testcaseSplinter Review
(In reply to comment #2)
> could you please attach a testcase so that I can play around with the patch?
> 

Sorry, I forgot to do it :).
Comment on attachment 218403 [details] [diff] [review]
patch

>diff -uNrap mozilla.orig/extensions/xforms/resources/content/select.xml mozilla/extensions/xforms/resources/content/select.xml
>--- mozilla.orig/extensions/xforms/resources/content/select.xml	2006-04-04 17:11:26.000000000 +0900
>+++ mozilla/extensions/xforms/resources/content/select.xml	2006-04-14 19:17:11.000000000 +0900
>@@ -73,6 +73,14 @@
> 
>     isItemSelected(item) - return true if item is selected
>       @param item - item control returned by appendItem()
>+
>+    getFreeEntryValues() - return space delimited list of free entry items values
>+
>+    allowFreeEntry(allowed) - enable/disable free enrty
>+      @param allowed - boolean value

nit: spelling s/enrty/entry

>+
>+    appendFreeEntryItem(value) - append and select free entry item
>+      @param value - item value
> -->
> 
> <bindings id="xformsSelectBindings"
>@@ -185,9 +193,20 @@
>         </getter>
>       </property>
> 
>-      <property name="selection"
>-                onget="return this.getAttribute('selection'); "
>-                onset="this.setAttribute('selection', val); "/>
>+      <property name="selection">
>+        <getter>
>+          return this.getAttribute("selection");
>+        </getter>
>+        <setter>
>+          if (val == "open")
>+            this.setAttribute("selection", "open");
>+            this.control.allowFreeEnry(true);
>+          } else {
>+            this.removeAttribute("selection");
>+            this.control.allowFreeEnry(false);
>+          }
>+        </setter>
>+      </property>
> 

I would think that you should set the attribute to 'val' no matter what it is since you have no idea what other values a custom control author might want to use here.  But you can still use the test for 'open' to determine whether to allowFreeEntry or not.

Otherwise the patch looks good.

Also, since the select1 is incremental by default, as we type in the select1, the bound value should change (like for a xhtml xforms select1).  Currently this patch doesn't provide that for XUL.  Do you want to provide that in this patch or do you want to do that in a seperate bug?
Attached patch patch2 (obsolete) — Splinter Review
(In reply to comment #4)

> nit: spelling s/enrty/entry
Fixed

> I would think that you should set the attribute to 'val' no matter what it is
> since you have no idea what other values a custom control author might want to
> use here.  But you can still use the test for 'open' to determine whether to
> allowFreeEntry or not.

Let it will be.

> 
> Also, since the select1 is incremental by default, as we type in the select1,
> the bound value should change (like for a xhtml xforms select1).  Currently
> this patch doesn't provide that for XUL.  Do you want to provide that in this
> patch or do you want to do that in a seperate bug?
> 

Fixed.
Attachment #218403 - Attachment is obsolete: true
Attachment #218765 - Flags: review?(aaronr)
Attachment #218403 - Flags: review?(aaronr)
Attachment #218765 - Flags: review?(doronr)
Attachment #218765 - Flags: review?(aaronr) → review+
Attached patch updatedSplinter Review
Updated patch to trunk
Attachment #218765 - Attachment is obsolete: true
Attachment #221660 - Flags: review?(doronr)
Attachment #218765 - Flags: review?(doronr)
Comment on attachment 221660 [details] [diff] [review]
updated

>diff -uNrap mozilla.orig/extensions/xforms/resources/content/select.xml mozilla/extensions/xforms/resources/content/select.xml
>--- mozilla.orig/extensions/xforms/resources/content/select.xml	2006-04-18 17:51:45.000000000 +0900
>+++ mozilla/extensions/xforms/resources/content/select.xml	2006-05-11 12:13:51.000000000 +0900
>@@ -73,6 +73,14 @@
> 
>     isItemSelected(item) - return true if item is selected
>       @param item - item control returned by appendItem()
>+
>+    getFreeEntryValues() - return space delimited list of free entry items values
>+
>+    allowFreeEntry(allowed) - enable/disable free entry
>+      @param allowed - boolean value
>+
>+    appendFreeEntryItem(value) - append and select free entry item
>+      @param value - item value
> -->
> 
> <bindings id="xformsSelectBindings"
>@@ -185,9 +193,19 @@
>         </getter>
>       </property>
> 
>-      <property name="selection"
>-                onget="return this.getAttribute('selection'); "
>-                onset="this.setAttribute('selection', val); "/>
>+      <property name="selection">
>+        <getter>
>+          return this.getAttribute("selection");
>+        </getter>
>+        <setter>
>+          this.setAttribute("selection", val);
>+
>+          if (val == "open")
>+            this.control.allowFreeEnry(true);
>+          else
>+            this.control.allowFreeEnry(false);
>+        </setter>
>+      </property>
> 
>       <method name="_buildSelect">
>         <body>
>@@ -230,8 +248,7 @@
>           for (var index in this._defaultHash) {
>             if (this._defaultHash[index].hits == 0) {
>               if (this.selection == 'open') {
>-                // XXX: If the select is open, the missing elements should be added
>-                // and selected per 8.1.10 in the spec.
>+                this.control.appendFreeEntryItem(index);
>               } else {
>                 // XXX: some of default values not found, we need to throw an
>                 // xforms-out-of-range event, but only if the select is 'closed'.
>@@ -594,11 +611,6 @@
>         <parameter name="aIsACopyItemSelectedOrDeselected"/>
>         <body>
>         <![CDATA[
>-          var selectedValues = "";
>-
>-          // select if found, unselect if not
>-          var options = this._controlArray;
>-
>           if (aIsACopyItemSelectedOrDeselected) {
>             aIsACopyItemSelectedOrDeselected.value = false;
>           }
>@@ -621,6 +633,13 @@
>             this._dispatchSelectEvents();
>             return;
>           }
>+
>+          var selectedValues = "";
>+          if (this.selection == "open") {
>+            selectedValues = this.control.getFreeEntryValues();
>+          }
>+
>+          var options = this._controlArray;
>           var boundType = boundNode.nodeType;
>           var copyNode;
> 
>@@ -871,6 +890,33 @@
>         </body>
>       </method>
> 
>+      <!-- Return values of free entry. The method is a stub and it should
>+        be removed when all native controls will support @selection="open".
>+      -->
>+      <method name="getFreeEntryValues">
>+        <body>
>+          return "";
>+        </body>
>+      </method>
>+
>+      <!-- Allow free entry. The method is a stub and it should be removed when
>+        all native controls will support @selection="open".
>+      -->
>+      <method name="allowFreeEntry">
>+        <parameter name="aAllowed"/>
>+        <body>
>+        </body>
>+      </method>
>+
>+      <!-- Append and select free entry item. The method is a stub and it should
>+        be removed when all native controls will support @selection="open".
>+      -->
>+      <method name="appendFreeEntryItem">
>+        <parameter name="aValue"/>
>+        <body>
>+        </body>
>+      </method>
>+
>       <property name="XFORMS_NS" readonly="true"
>                 onget="return 'http://www.w3.org/2002/xforms';"/>
> 
>@@ -883,6 +929,13 @@
>       <property name="XBL_NS" readonly="true"
>                 onget="return 'http://www.mozilla.org/xbl';"/>
> 
>+      <constructor>
>+        if (this.parentControl.selection == "open")
>+          this.allowFreeEntry(true);
>+        else
>+          this.allowFreeEntry(false);
>+      </constructor>
>+
>     </implementation>
>   </binding>
> 
>diff -uNrap mozilla.orig/extensions/xforms/resources/content/select-xul.xml mozilla/extensions/xforms/resources/content/select-xul.xml
>--- mozilla.orig/extensions/xforms/resources/content/select-xul.xml	2006-04-04 17:07:12.000000000 +0900
>+++ mozilla/extensions/xforms/resources/content/select-xul.xml	2006-05-11 12:13:51.000000000 +0900
>@@ -442,9 +442,42 @@
>           return aItem.getAttribute("selected") == "true";
>         </body>
>       </method>
>+
>+      <method name="getFreeEntryValues">
>+        <body>
>+          if (this.control.getAttribute("editable") == "true") {
>+            if (!this.control.selectedItem)
>+              return this.control.value;
>+            return "";
>+          }
>+        </body>
>+      </method>
>+
>+      <method name="allowFreeEntry">
>+        <parameter name="aAllowed"/>
>+        <body>
>+          if (aAllowed)
>+            this.control.setAttribute("editable", "true");
>+          else
>+            this.control.removeAttribute("editable");
>+        </body>
>+      </method>
>+
>+      <method name="appendFreeEntryItem">
>+        <parameter name="aValue"/>
>+        <body>
>+          if (this.control.getAttribute("editable") == "true") {
>+            this.control.value = aValue;
>+          }
>+        </body>
>+      </method>
>     </implementation>
> 
>     <handlers>
>+      <handler event="input">
>+        this.updateInstanceData(true);
>+      </handler>
>+
>       <handler event="focus" phase="capturing">
>         this.dispatchDOMUIEvent("DOMFocusIn");
>       </handler>
>@@ -460,9 +493,10 @@
> 
>       <!--
>         XXX: Since xforms:label can include arbitrary elements then 'focus',
>-          'blur' and 'command' events should be listen from 'xul:menulist'
>-          element only. The described problem is not actual now. Note we will
>-          get it when we will use node instead of text content for a label.
>+          'blur', 'command' and 'input' events should be listen from
>+          'xul:menulist' element only. The described problem is not actual now.
>+          Note we will get it when we will use node instead of text content for
>+          a label.
>       -->
>     </handlers>
>   </binding>
Attachment #221660 - Flags: review?(doronr) → review+
Fixed on trunk
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
Keywords: fixed1.8.1
Keywords: fixed1.8.0.5
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.