521246 - use toolkit's numberbox and spinbuttons widgets

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Raphaël Fairise]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.3) Gecko/20090910 Ubuntu/9.04 (jaunty) Shiretoko/3.5.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.4pre) Gecko/20090915 Thunderbird/3.0b4

Without XForms add-on, the value property of a XUL:textbox with type=number has string data type.
This is correct according to https://developer.mozilla.org/en/XUL/Property/Textbox.value

With XForms add-on installed, the data type of the value property of a XUL:textbox with type=number is changed to "number".

This breaks code where textbox's value is used as a string.
I've identified at least one case in Thunderbird:
Preferences->Composition->Addressing->Edit Directories->Add
"LDAP Directory Servers" window
The "port number" field is checked with a match() function that exists only with strings.
When using XForms add-on, an exception is thrown.
"Internal error in pref-directory-add.js:onAccept() TypeError: number.match is not a function"


I think that the issue is in xforms/resources/content/widgets-xul.xml
Code has been copied from mozilla/toolkit/content/widgets/numberbox.xml but, I don't know why, valueNumber property has disappeared and value property returns now a number.



Proposed patch following.

Reproducible: Always

Comment 1

[Raphaël Fairise]

Attachment: Patch for textbox value

Comment 2

[aaronr]

Comment on attachment 405273 [details] [diff] [review]
Patch for textbox value

Alex, you know XUL the best, probably.  Can you review the bug and the patch?

Comment 3

[alexander :surkov (:asurkov)]

Gecko 1.9 has numberbox widget and xforms hg sources are targeted to the latest version of Gecko. Therefore we don't need to keep a copy of the widget in xforms any more. Please update the patch.

The bug is targeted to 1.9.1.3. Do you need the fix for builds prior to Gecko 1.9?

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #3)
> 
> The bug is targeted to 1.9.1.3. Do you need the fix for builds prior to Gecko
> 1.9?

Just reread the bug description, and it seems you don't need since it hasn't known affects to xforms processor. The problem is we replace toolkit's textbox@type="number" widget on xforms one.

Comment 5

[Raphaël Fairise]

(In reply to comment #3)
> Gecko 1.9 has numberbox widget and xforms hg sources are targeted to the latest
> version of Gecko. Therefore we don't need to keep a copy of the widget in
> xforms any more. Please update the patch.

I'm not sure to understand what you expect.

> The bug is targeted to 1.9.1.3. Do you need the fix for builds prior to Gecko
> 1.9?

Yes. Same problem with FF 2.0.0.20 + XForms 0.8.6ff2.
textbox@type="number" XUL widget is modified by XForms add-on.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> (In reply to comment #3)
> > Gecko 1.9 has numberbox widget and xforms hg sources are targeted to the latest
> > version of Gecko. Therefore we don't need to keep a copy of the widget in
> > xforms any more. Please update the patch.
> 
> I'm not sure to understand what you expect.

I think all you should do is to remove xforms version of numberbox widget entirerly.

> > The bug is targeted to 1.9.1.3. Do you need the fix for builds prior to Gecko
> > 1.9?
> 
> Yes. Same problem with FF 2.0.0.20 + XForms 0.8.6ff2.
> textbox@type="number" XUL widget is modified by XForms add-on.

That's interesting because I thought numberbox was introduced a bit later than Firefox 2.0 (which is based on Gecko 1.8 iirc).

Comment 7

[aaronr]

Comment on attachment 405273 [details] [diff] [review]
Patch for textbox value

Sorry, accidentally marked this as approved.  Re-asking for review from Alex.

Comment 8

[alexander :surkov (:asurkov)]

Somebody should update the patch to remove numberbox widget completely from XForms extension.

Comment 9

[alexander :surkov (:asurkov)]

Sergey, can you look at this as well?

Comment 10

[Sergey Reymerov]

Attachment: remove numberbox

I made patch for that. Also I don't know who can review this patch except Alexand Surkov.

Comment 11

[alexander :surkov (:asurkov)]

Thank you for the patch. It sounds you can remove spinbuttons binding too. How did you test it?

Comment 12

[Alexey Gaidukov]

Sergey, some tests for spinbuttons you can find in bug 353880

Comment 13

[Sergey Reymerov]

Attachment: remove numberbox and spinbutton

I've removed spinbutton and numberbox. 
Also for testing I add lines to xforms.css (else my test doesn't work):
html|*:root input[mozType|typelist~="http://www.w3.org/2001/XMLSchema#integer"],
html|*:root input[mozType|typelist~="http://www.w3.org/2001/XMLSchema#int"],
...
Is it correct?

Comment 14

[Sergey Reymerov]

Attachment: test for numberbox

This test I use for patch. 

Also I've catch new bug. Value in <data2> sets in <xf:input> but when I change it by spinbuttons it become 1 or -1. It looks like that numberbox or spinbutton internal value are 0 (zero).

Comment 15

[alexander :surkov (:asurkov)]

(In reply to comment #14)

> Also I've catch new bug. Value in <data2> sets in <xf:input> but when I change
> it by spinbuttons it become 1 or -1. It looks like that numberbox or spinbutton
> internal value are 0 (zero).

Please file new bug for this.

Comment 16

[alexander :surkov (:asurkov)]

reassigning to Sergey since he works on it currently.

Comment 17

[alexander :surkov (:asurkov)]

Comment on attachment 413527 [details] [diff] [review]
remove numberbox and spinbutton

thanks, nice work.

Comment 18

[alexander :surkov (:asurkov)]

changing a summary to better reflect the bug

Comment 19

[alexander :surkov (:asurkov)]

landed - http://hg.mozilla.org/xforms/rev/fa67d69c9495