327234 - Expose abstract interface for input[type="gMonth"] and input[type="gDay"]

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[alexander :surkov (:asurkov)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051212 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051212 Firefox/1.6a1

Patch is comming up.

Reproducible: Always

Comment 1

[alexander :surkov (:asurkov)]

Attachment: test case

Comment 2

[alexander :surkov (:asurkov)]

Attachment: patch

was exposed interface for inputs type="gMonth" and type "gDay", also all input controls was moved into separate file. The cause is too big files.

Comment 3

[alexander :surkov (:asurkov)]

Attachment: split out patch

Comment 4

[Allan Beaufour]

(In reply to comment #3)
> Created an attachment (id=212099) [edit]
> split out patch

I asked surkov to do the "split" in a seperate patch, as it makes checking the changes a lot easier.

Comment 5

[Allan Beaufour]

Comment on attachment 212099 [details] [diff] [review]
split out patch

r=me

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 212099 [details] [diff] [review]
split out patch


>+<bindings id="xformsBindings"

Not all our bindings files should have the same id for their root element ;)
I think there is same problem also with select.xml

>+          xmlns="http://www.mozilla.org/xbl"
>+          xmlns:html="http://www.w3.org/1999/xhtml"
>+          xmlns:xbl="http://www.mozilla.org/xbl"
>+          xmlns:xforms="http://www.w3.org/2002/xforms"
>+          xmlns:lazy="http://www.mozilla.org/projects/xforms/2005/lazy">

I think xmlns:lazy is not needed here.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)

> Not all our bindings files should have the same id for their root element ;)
> I think there is same problem also with select.xml

Ok, I'll check namespaces and remove unused of them in second patch :)

Comment 8

[alexander :surkov (:asurkov)]

Why input month and day doesn't handle focus and blur and doesn't fire DOMFocusIn and DOMFocusOut events like it do other input controls? For what is DOMFocusIn and DOMFocusOut events firing needed at all?

Comment 9

[Allan Beaufour]

(In reply to comment #7)
> (In reply to comment #6)
> 
> > Not all our bindings files should have the same id for their root element ;)
> > I think there is same problem also with select.xml
> 
> Ok, I'll check namespaces and remove unused of them in second patch :)

Agreed, just remember it in second patch.

Checked attachment 212099 [details] [diff] [review] in to trunk.

Comment 10

[alexander :surkov (:asurkov)]

Attachment: main patch

Comment 11

[Allan Beaufour]

(In reply to comment #8)
> Why input month and day doesn't handle focus and blur and doesn't fire
> DOMFocusIn and DOMFocusOut events like it do other input controls?

Well, they should.

> For what is DOMFocusIn and DOMFocusOut events firing needed at all?

http://www.w3.org/TR/2005/PER-xforms-20051006/index-all.html#evt-DOMFocusIn

Comment 12

[Doron Rosenberg (IBM)]

Comment on attachment 212203 [details] [diff] [review]
main patch

>+            XHTML_NS: 'http://www.w3.org/1999/xhtml',
>+
>+            set readonly(val) {
>+              this.disabled = val;
>+            },
>+            appendMonth: function(name, value) {
>+              var option = this.ownerDocument.
>+                createElementNS(this.XHTML_NS, 'option');
>+              option.textContent = name;
>+              option.setAttribute('value', value);
>+              this.appendChild(option);
>+            }
>+          };

Why not just use the XHTML_NS string directly, creating a property for one usage is silly.

>+
>+
>+  <!-- INPUT: DAY -->

Why two spaces?

>+            XHTML_NS: 'http://www.w3.org/1999/xhtml',
>+
>+            set readonly(val) {
>+              this.disabled = val;
>+            },
>+            appendDay: function(name, value) {
>+              var option = this.ownerDocument.
>+                createElementNS(this.XHTML_NS, 'option');
>+              option.textContent = name;
>+              option.setAttribute('value', value);
>+              this.appendChild(option);
>+            }

ditto for the xhtml ns

>+
>+
>   <!-- SECRET: <DEFAULT> -->

2 newlines again

>@@ -729,194 +727,139 @@
>     </handlers>
>   </binding>
> 
>+
>   <!-- INPUT: Month
and again

> 
>-      <method name="_change">
>+      <method name="updateInstanceData">
>+        <parameter name="incremental"/>

please call it aIncremental, so we know that it is an argument

>   </binding>
> 
>+

2 newlines

rest looks good

Comment 13

[alexander :surkov (:asurkov)]

(In reply to comment #12)

> Why not just use the XHTML_NS string directly, creating a property for one
> usage is silly.

The object is created one time but method appendMonth() is used many times. I just thought why should we create xmlns string every time when method is called?


> Why two spaces?
> 2 newlines again

I belive if two lines is presented between two binding then I can easier locate needing binding. One line used as separator inside of binding. Two lines are used between bindings. Btw xforms.xml contains two lines between bindings.

> please call it aIncremental, so we know that it is an argument

For sure, I didn't notice that.

If you don't agree with me about xmlns string and two lines then please let me know and I'll fix it.

Comment 14

[Allan Beaufour]

Comment on attachment 212203 [details] [diff] [review]
main patch

> +++ mozilla/extensions/xforms/resources/content/input-xhtml.xml	2006-02-17 18:53:06.000000000 +0800
>  <bindings id="xformsBindings"

Didn't you promise to change the id? :)

> +  <!-- INPUT: DAY -->

> +      <method name="getControlElement">
> +        <body>
> +          return {
> +            __proto__: this.ownerDocument.
> +              getAnonymousElementByAttribute(this, 'anonid', 'control'),
> +
> +            XHTML_NS: 'http://www.w3.org/1999/xhtml',
> +
> +            set readonly(val) {
> +              this.disabled = val;
> +            },
> +            appendDay: function(name, value) {
> +              var option = this.ownerDocument.
> +                createElementNS(this.XHTML_NS, 'option');
> +              option.textContent = name;
> +              option.setAttribute('value', value);
> +              this.appendChild(option);
> +            }
> +          };
> +        </body>
> +      </method>
> +
> +      <property name="XHTML_NS" readonly="true"
> +                onget="return 'http://www.w3.org/1999/xhtml';"/>

What's up with the two XHTML_NS definitions?

Comment 15

[alexander :surkov (:asurkov)]

Attachment: patch (allan comments)

Comment 16

[Allan Beaufour]

Checked in to trunk