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

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
11 years ago
a year ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

({fixed1.8.0.5, fixed1.8.1})

Trunk
fixed1.8.0.5, fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

11 years ago
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.
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

11 years ago
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.
Attachment #218403 - Flags: review?(aaronr)

Comment 2

11 years ago
could you please attach a testcase so that I can play around with the patch?
(Assignee)

Comment 3

11 years ago
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

11 years ago
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?
(Assignee)

Comment 5

11 years ago
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.
Attachment #218403 - Attachment is obsolete: true
Attachment #218765 - Flags: review?(aaronr)
Attachment #218403 - Flags: review?(aaronr)
(Assignee)

Updated

11 years ago
Attachment #218765 - Flags: review?(doronr)

Updated

11 years ago
Attachment #218765 - Flags: review?(aaronr) → review+
(Assignee)

Comment 6

11 years ago
Created attachment 221660 [details] [diff] [review]
updated

Updated patch to trunk
Attachment #218765 - Attachment is obsolete: true
Attachment #221660 - Flags: review?(doronr)
Attachment #218765 - Flags: review?(doronr)

Comment 7

11 years ago
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+

Comment 8

11 years ago
Fixed on trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch

Updated

11 years ago
Keywords: fixed1.8.1

Updated

11 years ago
Keywords: fixed1.8.0.5

Updated

11 years ago
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.