Last Comment Bug 332197 - support @selection="open" for xf:select1[appearance='minimal'] for xul
: support @selection="open" for xf:select1[appearance='minimal'] for xul
Status: RESOLVED FIXED
: fixed1.8.0.5, fixed1.8.1
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: alexander :surkov
: Stephen Pride
:
Mentors:
Depends on:
Blocks: 327236
  Show dependency treegraph
 
Reported: 2006-03-30 00:05 PST by alexander :surkov
Modified: 2016-07-15 14:46 PDT (History)
1 user (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (5.39 KB, patch)
2006-04-14 03:22 PDT, alexander :surkov
no flags Details | Diff | Splinter Review
testcase (783 bytes, patch)
2006-04-16 18:34 PDT, alexander :surkov
no flags Details | Diff | Splinter Review
patch2 (6.15 KB, patch)
2006-04-17 21:05 PDT, alexander :surkov
aaronr: review+
Details | Diff | Splinter Review
updated (6.16 KB, patch)
2006-05-10 20:15 PDT, alexander :surkov
doronr: review+
Details | Diff | Splinter Review

Description alexander :surkov 2006-03-30 00:05:12 PST
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.
Comment 1 alexander :surkov 2006-04-14 03:22:17 PDT
Created attachment 218403 [details] [diff] [review]
patch

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.
Comment 2 aaronr 2006-04-14 15:07:36 PDT
could you please attach a testcase so that I can play around with the patch?
Comment 3 alexander :surkov 2006-04-16 18:34:58 PDT
Created attachment 218625 [details] [diff] [review]
testcase

(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 4 aaronr 2006-04-17 12:49:04 PDT
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?
Comment 5 alexander :surkov 2006-04-17 21:05:42 PDT
Created attachment 218765 [details] [diff] [review]
patch2

(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.
Comment 6 alexander :surkov 2006-05-10 20:15:29 PDT
Created attachment 221660 [details] [diff] [review]
updated

Updated patch to trunk
Comment 7 Doron Rosenberg (IBM) 2006-05-12 08:09:42 PDT
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>
Comment 8 Allan Beaufour 2006-05-15 07:11:20 PDT
Fixed on trunk

Note You need to log in before you can comment on or make changes to this bug.