Last Comment Bug 316353 - Support range for other datatypes than numbers
: Support range for other datatypes than numbers
Status: RESOLVED FIXED
:
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Merle Sterling
: Stephen Pride
:
Mentors:
http://www.mozilla.org/projects/xforms/
Depends on: 331987 353880 370551 372736 372737 372739
Blocks: 322255 326372 326373 343525 353738
  Show dependency treegraph
 
Reported: 2005-11-14 00:46 PST by Allan Beaufour
Modified: 2016-07-15 14:46 PDT (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
xul number spinbutton sample (407 bytes, application/vnd.mozilla.xul+xml)
2006-08-31 13:55 PDT, aaronr
no flags Details
testcase - xsd:date (1.09 KB, text/xml)
2006-09-21 16:30 PDT, Merle Sterling
no flags Details
testcase - xsd:dateTime (1.16 KB, text/xml)
2006-09-21 16:31 PDT, Merle Sterling
no flags Details
testcase - xsd:time (1.06 KB, text/xml)
2006-09-21 16:32 PDT, Merle Sterling
no flags Details
testcase - xsd:gDay (1.06 KB, text/xml)
2006-09-21 16:33 PDT, Merle Sterling
no flags Details
testcase - xsd:gMonth (1.08 KB, text/xml)
2006-09-21 16:33 PDT, Merle Sterling
no flags Details
testcase - xsd:gMonthDay (1.11 KB, text/xml)
2006-09-21 16:34 PDT, Merle Sterling
no flags Details
testcase - xsd:gYear (1.05 KB, text/xml)
2006-09-21 16:34 PDT, Merle Sterling
no flags Details
testcase - xsd:gYearMonth (1.12 KB, text/xml)
2006-09-21 16:35 PDT, Merle Sterling
no flags Details
testcase - xforms:dayTimeDuration (1.15 KB, text/xml)
2006-09-21 16:36 PDT, Merle Sterling
no flags Details
testcase - xforms:yearMonthDuration (1.15 KB, text/xml)
2006-09-21 16:37 PDT, Merle Sterling
no flags Details
Bindings for new range types (87.82 KB, patch)
2006-09-22 15:00 PDT, Merle Sterling
surkov.alexander: review-
Details | Diff | Splinter Review
patch (73.89 KB, patch)
2006-09-29 18:05 PDT, Merle Sterling
no flags Details | Diff | Splinter Review
patch (76.42 KB, patch)
2006-10-02 14:46 PDT, Merle Sterling
no flags Details | Diff | Splinter Review
patch (90.70 KB, patch)
2006-10-13 21:50 PDT, Merle Sterling
no flags Details | Diff | Splinter Review
dateTime range (31.03 KB, patch)
2007-02-02 13:45 PST, Merle Sterling
no flags Details | Diff | Splinter Review

Description Allan Beaufour 2005-11-14 00:46:03 PST
The spec. states that range should support: xsd:duration, xsd:date, xsd:time, xsd:dateTime, xsd:gYearMonth, xsd:gYear, xsd:gMonthDay, xsd:gDay, xsd:gMonth, xsd:float, xsd:decimal, xsd:double
Comment 1 aaronr 2006-04-21 09:41:39 PDT
I don't think that this is an enhancement since it clearly says in the spec that these datatypes must be supported.
Comment 2 Allan Beaufour 2006-04-24 00:10:39 PDT
(In reply to comment #1)
> I don't think that this is an enhancement since it clearly says in the spec
> that these datatypes must be supported.

Spec. schmeck :) The spec. only says that we should not bind to anything else than these types ;)

Yes, you are right.
Comment 3 Merle Sterling 2006-08-29 12:43:59 PDT
With the exception of gDay, gMonth, and gYear (which work with the current code for numeric types) none of these types will work well with a regular slider control. 

Date and dateTime could perhaps use a calendar control but we would need to implement an entirely new control to restrict the range to fall between 'start' and 'end' and the notion of 'step' is not reallly applicable. If a step attribute is provided, we would have to interpret something like "0001-01-05" to mean 'step year by 1, step month by 1, step day by 5' (Steve Speicher's suggestion) and that is not really possible with a calendar control.

The remaining types would be best represented as a series of spin buttons, with one spin button per component of the type (eg. 'time' would have one spin button for hours(hh), one for minutes(mm), one for seconds(ss), and one for milliseconds(.sss)). date and dateTime will be easier to represent as spin buttons as well and we would have greater control over stepping thru the range. With spin buttons, we could interpret any provided step value for date as in the example above.

So the best approach for implementing support for all of these types is probably to implement a base spin button control and create a new binding for each of the supported range types using one or more spin buttons to represent the type.
Comment 4 aaronr 2006-08-29 14:37:19 PDT
I would also vote to ignore @step for any type not represented by a slider control for the moment.  Until we get a good use case.  It isn't clear to me how step should be specified for the different types, or how it would be used.  In the case of a dateTime, if I specify step="0001-01-02" would that mean that I'm not allowed to modify the hours, minutes and seconds?  Would that mean that I can only specify days in increments of 2?  If we don't implement @step for these types, though, we should probably open a placeholder bug for the functionality and some statement in the bug for the types of behavior that we need defined.
Comment 5 aaronr 2006-08-31 13:55:36 PDT
Created attachment 236298 [details]
xul number spinbutton sample

I found this numberbox spinbutton-type widget in mozilla.  Seems to work well.  Might be useful to study the code behind it and reuse it where possible.
Comment 6 aaronr 2006-08-31 14:14:02 PDT
(In reply to comment #5)
> Created an attachment (id=236298) [edit]
> xul number spinbutton sample
> 
> I found this numberbox spinbutton-type widget in mozilla.  Seems to work well. 
> Might be useful to study the code behind it and reuse it where possible.
> 

Only works on trunk, so if we want this to work on branches, it won't be as simple as just pointing to the binding.
Comment 7 Merle Sterling 2006-09-21 16:30:50 PDT
Created attachment 239573 [details]
testcase - xsd:date
Comment 8 Merle Sterling 2006-09-21 16:31:29 PDT
Created attachment 239575 [details]
testcase - xsd:dateTime
Comment 9 Merle Sterling 2006-09-21 16:32:22 PDT
Created attachment 239576 [details]
testcase - xsd:time
Comment 10 Merle Sterling 2006-09-21 16:33:03 PDT
Created attachment 239577 [details]
testcase - xsd:gDay
Comment 11 Merle Sterling 2006-09-21 16:33:43 PDT
Created attachment 239578 [details]
testcase - xsd:gMonth
Comment 12 Merle Sterling 2006-09-21 16:34:14 PDT
Created attachment 239579 [details]
testcase - xsd:gMonthDay
Comment 13 Merle Sterling 2006-09-21 16:34:50 PDT
Created attachment 239580 [details]
testcase - xsd:gYear
Comment 14 Merle Sterling 2006-09-21 16:35:41 PDT
Created attachment 239581 [details]
testcase - xsd:gYearMonth
Comment 15 Merle Sterling 2006-09-21 16:36:28 PDT
Created attachment 239582 [details]
testcase - xforms:dayTimeDuration
Comment 16 Merle Sterling 2006-09-21 16:37:06 PDT
Created attachment 239583 [details]
testcase - xforms:yearMonthDuration
Comment 17 Merle Sterling 2006-09-22 15:00:49 PDT
Created attachment 239714 [details] [diff] [review]
Bindings for new range types

Created bindings for the new range types covered by the attached testcases. Each type is implemented using one or more spinbuttons for each component of the type; eg xsd:date is implemented using 3 spinbuttons - one each for years, months, and days.

Further enhancements to the spinbuttons and the types using them will be handled in another bug as this patch only addresses the required functionality to implement a range control for each of the new types.
Comment 18 alexander :surkov 2006-09-23 01:27:05 PDT
Comment on attachment 239714 [details] [diff] [review]
Bindings for new range types

I will do some comments below about code styling and general issues. Also I'd vote to do the patch after bug 352462 since changes are intersected. Also I will be happy if you can split the patch on several if it's possible.

>? resources/content/range-date.xml
>? resources/content/range-datetime.xml
>? resources/content/range-duration.xml
>? resources/content/range-gregorian.xml
>? resources/content/range-time.xml

You should use cvsdo to avoid this. On Windows OS I will do:

cd d:\cygwin\bin
perl cvsdo add /cygdrive/d/mozilla/mozilla/extensions/xforms/resources/content/range-date.xml

>+  content/xforms/range-date.xml                (resources/content/range-date.xml)
>+  content/xforms/range-time.xml                (resources/content/range-time.xml)
>+  content/xforms/range-datetime.xml            (resources/content/range-datetime.xml)
>+  content/xforms/range-gregorian.xml           (resources/content/range-gregorian.xml)
>+  content/xforms/range-duration.xml            (resources/content/range-duration.xml)

I'd prefer to put new bindings into range-xhtml.xml file if the bindings are not very big. But in any way if your widgets are for xhtml context then your file should have '-xhtml' postfix. It will be consistent with other xforms code. Also I don't see in your bindings where do you use "start", "end", "step" attributes.

>+  /* range type="xsd:int" */
>+html|*:root range[mozType|type="http://www.w3.org/2001/XMLSchema#int"] {
>+  -moz-binding: url('chrome://xforms/content/range.xml#xformswidget-range');
>+}

You should use @typelist attribute. I'm not sure but since xsd:int are derived from xsd:decimal then xsd:int binding isn't used. That's valid only if @typelist attirbute contains base type for shema spec definded types.

But in any way I prefer to see:

>+html|*:root range[mozType|type="http://www.w3.org/2001/XMLSchema#int"],
>+html|*:root range[mozType|type="http://www.w3.org/2001/XMLSchema#float"] {
>+  -moz-binding: url('chrome://xforms/content/range.xml#xformswidget-range');
>+}

And so one. The same is true for spinbuttons bindings. Also please use mozType:type attribute to add binding for spinbutton instead @class.

>+   - The Initial Developer of the Original Code is
>+   - Novell, Inc.

That's news for me :) Your spy essence was discovered!

>+   - Contributor(s):
>+   - Merle Sterling <msterlin@us.ibm.com>

I guess you can add '(original author)' after email.

>+      <property name="XHTML_NS" readonly="true"
>+                onget="return 'http://www.w3.org/1999/xhtml';"/>

Probably that's not very nice but I'd like to move it into 'xformswidget-base'.

>+      <method name="refresh">
>+        <body>
>+          <![CDATA[

What about readonly support?

>+      <method name="focus">
>+        <body>
>+          return true;
>+        </body>
>+      </method>

What about this?


>+      
>+  <binding id="xformswidget-range-gYearMonth"
>+           extends="chrome://xforms/content/xforms.xml#xformswidget-base">

Every binding should have comment before its declaration. Look at other our bindings.
Comment 19 alexander :surkov 2006-09-23 09:30:03 PDT
Comment on attachment 239714 [details] [diff] [review]
Bindings for new range types

Firstly I'm not sure that range widget should have look of spin control. Specs says: In graphical environments, this form control may be rendered as a "slider" or "rotary control". I don't know what is "rotary control" though. Also specs say: Implementations should inform the user of the upper and lower bounds, as well as the step size, if any. I don't see how your controls informs about it.
Second, if you using spin controls for range then 1) it's better to steal spinbuttons widget from toolkit since they are on trunk only I guess and 2) spin buttons should be placed together (increment on top, decrement on bottom) since it's more comfortable.
Comment 20 alexander :surkov 2006-09-23 09:44:28 PDT
(In reply to comment #18)

> You should use @typelist attribute. I'm not sure but since xsd:int are derived
> from xsd:decimal then xsd:int binding isn't used. That's valid only if
> @typelist attirbute contains base type for shema spec definded types.

Right, if type is xsd:integer then @typelist attribute is "xsd:integer xsd:decimal". Therefore you need css rule for xsd:decimal only when you will use @typelist attribute.

> Also please use
> mozType:type attribute to add binding for spinbutton instead @class.

Sorry, not mozType:type, please use mozType:spinbutton or something like.

But firstly (before you will update your patch) we should decide how the control should looks. I'd don't like a lot to have usual textfield with spinbuttons for range controls. But again, I don't know how they should look.
Comment 21 alexander :surkov 2006-09-23 12:52:34 PDT
(In reply to comment #18)

> Also I don't see in your bindings where do you use "start", "end", "step"
> attributes.

Never mind, I can see.

> Probably that's not very nice but I'd like to move it into 'xformswidget-base'.

Probably should be in 'xformswidget-general'.

> >+      <method name="focus">
> >+        <body>
> >+          return true;
> >+        </body>
> >+      </method>
> 
> What about this?

At least why do you return true if method is actually not implemented?

(In reply to comment #19)

> Second, if you using spin controls for range then 1) it's better to steal
> spinbuttons widget from toolkit since they are on trunk only I guess

Even if you posted bug 353880 and spinbuttons on trunk are only for xul then I guess it's good to try to steal them or at least to try to place them in more friendly way.
Comment 22 aaronr 2006-09-24 18:53:34 PDT
(In reply to comment #19)
> (From update of attachment 239714 [details] [diff] [review] [edit])
> Firstly I'm not sure that range widget should have look of spin control. Specs
> says: In graphical environments, this form control may be rendered as a
> "slider" or "rotary control". I don't know what is "rotary control" though.

Rotary control is like the volume knob on a radio.  Circular and when spun clockwise, values increase.  When counter-clockwise, values decrease.

> Also specs say: Implementations should inform the user of the upper and lower
> bounds, as well as the step size, if any. I don't see how your controls informs
> about it.

Yes, eventually we'll have to have a lower limit on one side and upper limit on the other side of the set of spin buttons.  We chose spin buttons since it is easier for the user to get exactly the value that they want, especially when 'step' has very little meanging outside of numerical types.  For example, if you have a beginning date and an end date that are 100 years apart, it would take a long time to move a slider to the correct day somewhere in the middle when there are 36500 days in between the beginning and end.

> Second, if you using spin controls for range then 1) it's better to steal
> spinbuttons widget from toolkit since they are on trunk only I guess and 2)
> spin buttons should be placed together (increment on top, decrement on bottom)
> since it's more comfortable.
> 

This will be a multi-step process.  Multiple patches.  First is to get the functionality and usability agreed upon and checked in.  What things look like will need to go in on a seperate bug.  We'd be very happy to get our spin buttons to look as good as they do in XUL.
Comment 23 alexander :surkov 2006-09-24 19:58:54 PDT
(In reply to comment #22)

> > Also specs say: Implementations should inform the user of the upper and lower
> > bounds, as well as the step size, if any. I don't see how your controls informs
> > about it.
> 
> Yes, eventually we'll have to have a lower limit on one side and upper limit on
> the other side of the set of spin buttons.

How about @size?

>  We chose spin buttons since it is
> easier for the user to get exactly the value that they want, especially when
> 'step' has very little meanging outside of numerical types.  For example, if
> you have a beginning date and an end date that are 100 years apart, it would
> take a long time to move a slider to the correct day somewhere in the middle
> when there are 36500 days in between the beginning and end.

I understand slider or rotary controls are not always useful. The another thing confuses me. Now input that is bound to number data type looks like spin control in xul context. That means range and input are almost one and the same for some cases. I guess it's very free interpretation of specification. And therefore I'd prefer to have spin control rather for input than for range. Also that allows us to be care to specs.
 
> > Second, if you using spin controls for range then 1) it's better to steal
> > spinbuttons widget from toolkit since they are on trunk only I guess and 2)
> > spin buttons should be placed together (increment on top, decrement on bottom)
> > since it's more comfortable.
> > 
> 
> This will be a multi-step process.  Multiple patches.  First is to get the
> functionality and usability agreed upon and checked in.  What things look like
> will need to go in on a seperate bug.  We'd be very happy to get our spin
> buttons to look as good as they do in XUL.
> 

I can assume it's multi-step process. But firstly I'd prefer to have one good control instead many non-ended. Second, I guess it's better to modify existing things instead of writing new, more over if new things are not very comfortable.
Comment 24 alexander :surkov 2006-09-24 20:56:00 PDT
I'd vote to use spin controls for input that is bound to types Merle uses. It will be consistent with input[xsd:integer] for xul. @start/@end attributes we can handle from schema, though I don't know how to specify the @step.

But if we cannot handle @step via schema and you'd like to save spin controls for range in any way. 1) Then I guess we should remove spin controls for xul's input[xsd:integer]. 2) we should name these ranges as range[appearance='compact'] or something else to have both range presentations (spin-like and slider-like). Note, we should have spin-like range for number types too. 3) Also we should have slider-like range for all these types like we have it for numbers type now.
Comment 25 alexander :surkov 2006-09-24 21:07:02 PDT
(In reply to comment #4)
> I would also vote to ignore @step for any type not represented by a slider
> control for the moment.  Until we get a good use case.  It isn't clear to me
> how step should be specified for the different types, or how it would be used. 
> In the case of a dateTime, if I specify step="0001-01-02" would that mean that
> I'm not allowed to modify the hours, minutes and seconds?  Would that mean that
> I can only specify days in increments of 2?  If we don't implement @step for
> these types, though, we should probably open a placeholder bug for the
> functionality and some statement in the bug for the types of behavior that we
> need defined.
> 

Sorry, I missed your previous discussion :).
And if @step is supposed to be missed then I can't see no one reason why don't preserve spin controls for xforms input excepting the reason that @start/@end is more friendly than schema magic. But again we are in the spec wholly: 1) we don't miss @step attribute (since input hasn't it), 2) range looks like slider control.
Comment 26 Merle Sterling 2006-09-25 18:03:41 PDT
The patch for this bug shares some code with bug 331987, so making this bug depend on 331987.
Comment 27 aaronr 2006-09-27 16:15:01 PDT
(In reply to comment #25)
> (In reply to comment #4)
> > I would also vote to ignore @step for any type not represented by a slider
> > control for the moment.  Until we get a good use case.  It isn't clear to me
> > how step should be specified for the different types, or how it would be used. 
> > In the case of a dateTime, if I specify step="0001-01-02" would that mean that
> > I'm not allowed to modify the hours, minutes and seconds?  Would that mean that
> > I can only specify days in increments of 2?  If we don't implement @step for
> > these types, though, we should probably open a placeholder bug for the
> > functionality and some statement in the bug for the types of behavior that we
> > need defined.
> > 
> 
> Sorry, I missed your previous discussion :).
> And if @step is supposed to be missed then I can't see no one reason why don't
> preserve spin controls for xforms input excepting the reason that @start/@end
> is more friendly than schema magic. But again we are in the spec wholly: 1) we
> don't miss @step attribute (since input hasn't it), 2) range looks like slider
> control.
> 

You want to use a spin button for a xf:input bound to a node with a numerical type?  My only issue with that would behave differently than the other XForms processors on a control that would be OFTEN used (as opposed to date bound ranges which I doubt will be very common).  If you want to create a binding that a mozilla user can use via CSS instead of our default binding, I have no problem with that.  Or maybe hook it into with a xf:input with @appearance="full".
Comment 28 alexander :surkov 2006-09-28 08:47:57 PDT
(In reply to comment #27)

> You want to use a spin button for a xf:input bound to a node with a numerical
> type?  My only issue with that would behave differently than the other XForms
> processors on a control that would be OFTEN used (as opposed to date bound
> ranges which I doubt will be very common).  If you want to create a binding
> that a mozilla user can use via CSS instead of our default binding, I have no
> problem with that.  Or maybe hook it into with a xf:input with
> @appearance="full".
> 

I'm not sure on 100% I like to see spin widget as xforms input presentation. Just input[xsd:integer] for xul have a look of spin control since it contains xul:textbox@type="number". If we decide that spin controls is for range than we should not use spin for input. That's all. How other xforms processors shows range controls?
Comment 29 aaronr 2006-09-28 14:15:55 PDT
(In reply to comment #28)
> (In reply to comment #27)
> 
> > You want to use a spin button for a xf:input bound to a node with a numerical
> > type?  My only issue with that would behave differently than the other XForms
> > processors on a control that would be OFTEN used (as opposed to date bound
> > ranges which I doubt will be very common).  If you want to create a binding
> > that a mozilla user can use via CSS instead of our default binding, I have no
> > problem with that.  Or maybe hook it into with a xf:input with
> > @appearance="full".
> > 
> 
> I'm not sure on 100% I like to see spin widget as xforms input presentation.
> Just input[xsd:integer] for xul have a look of spin control since it contains
> xul:textbox@type="number". If we decide that spin controls is for range than we
> should not use spin for input. That's all. How other xforms processors shows
> range controls?
> 

usually sliders for the types that they support, which are mostly the numerical types.  formsPlayer doesn't support all of the types that we eventually will, but users sliders for the types that they do support.  XSmiles uses slider for numerical types and combines slider with calendar for some date types, I believe.  Might be nice to support that at some point (maybe for @appearance="full").  But that isn't trivial so we are doing spinbuttons first.  To do that we'd probably have to refactor datepicker to some degree.
Comment 30 Merle Sterling 2006-09-29 18:05:46 PDT
Created attachment 240694 [details] [diff] [review]
patch

New patch: 
- new bindings now extend from xformswidget-range-base.
- Fixed the issues mentioned by Surkov.
Comment 31 alexander :surkov 2006-09-29 18:58:11 PDT
Comment on attachment 240694 [details] [diff] [review]
patch


>+  content/xforms/range-date-xhtml.xml          (resources/content/range-date-xhtml.xml)
>+  content/xforms/range-datetime-xhtml.xml      (resources/content/range-datetime-xhtml.xml)
>+  content/xforms/range-duration-xhtml.xml      (resources/content/range-duration-xhtml.xml)
>+  content/xforms/range-gregorian-xhtml.xml     (resources/content/range-gregorian-xhtml.xml)
>+  content/xforms/range-time-xhtml.xml          (resources/content/range-time-xhtml.xml)

Merle, do you think about xul controls support? I belive your bindings can be splitted on two, one is base bingind, other is xhtml binding. Probably in this case you don't need in huge amount of new files.

>+  <!-- The file contains the implementation of xforms range element for type
>+       xsd:date.

You should mention here you use spin representation.

>+          this.monthSpin = document.getAnonymousElementByAttribute(this, "anonid", "monthSpin");
>+          this.daySpin = document.getAnonymousElementByAttribute(this, "anonid", "daySpin");
>+          this.yearSpin = document.getAnonymousElementByAttribute(this, "anonid", "yearSpin");

That's not usual practice. Use the next template:

<property name="monthSpin" readonly="true">
  <getter>
    if (!this._monthSpin) {
      this._monthSpin = this.onwerDocument.getAnonymousElementByAttribute(this, "anonid", "monthSpin");
    }
    return this._monthSpin;
  </getter>
</property>
<field name="_monthSpin">null</field>
Comment 32 alexander :surkov 2006-09-29 19:03:17 PDT
Comment on attachment 240694 [details] [diff] [review]
patch


>+ /* range type="xsd:date" */
>+html|*:root range[mozType|typelist~="http://www.w3.org/2001/XMLSchema#date"] html|span[mozType|spinbutton] {
>+  -moz-binding: url('chrome://xforms/content/widgets-xhtml.xml#spinbutton');
>+}
>+
>+ /* range type="xsd:dateTime" */
>+html|*:root range[mozType|typelist~="http://www.w3.org/2001/XMLSchema#dateTime"] html|span[mozType|spinbutton] {
>+  -moz-binding: url('chrome://xforms/content/widgets-xhtml.xml#spinbutton');
>+}
>+
>+ /* range type="xsd:time" */
>+html|*:root range[mozType|typelist~="http://www.w3.org/2001/XMLSchema#time"] html|span[mozType|spinbutton] { 
>+  -moz-binding: url('chrome://xforms/content/widgets-xhtml.xml#spinbutton');
>+}

I dont like a much many spinbutton bindings, please do:

html|*:root range[mozType|typelist~="http://www.w3.org/2001/XMLSchema#date"] html|span[mozType|spinbutton],
html|*:root range[mozType|typelist~="http://www.w3.org/2001/XMLSchema#dateTime"] html|span[mozType|spinbutton],
html|*:root range[mozType|typelist~="http://www.w3.org/2001/XMLSchema#time"] html|span[mozType|spinbutton] {
  -moz-binding: url('chrome://xforms/content/widgets-xhtml.xml#spinbutton');
}

and etc.
Comment 33 alexander :surkov 2006-09-29 23:25:41 PDT
(In reply to comment #29)
> usually sliders for the types that they support, which are mostly the numerical
> types.  formsPlayer doesn't support all of the types that we eventually will,
> but users sliders for the types that they do support.  XSmiles uses slider for
> numerical types and combines slider with calendar for some date types, I
> believe.  Might be nice to support that at some point (maybe for
> @appearance="full").  But that isn't trivial so we are doing spinbuttons first.
>  To do that we'd probably have to refactor datepicker to some degree.
> 

Ok, in general spin widget looks good for range since spin supports start/end/step. But I'd propose to use for example appearance="minimal" for spin presentation and don't do spin presentation available by default. By default I think it's better to use slider control, it will be go with specs. Then we should stop spin control using for xforms input controls. What do you think?
Comment 34 aaronr 2006-10-01 16:11:27 PDT
(In reply to comment #33)
> (In reply to comment #29)
> > usually sliders for the types that they support, which are mostly the numerical
> > types.  formsPlayer doesn't support all of the types that we eventually will,
> > but users sliders for the types that they do support.  XSmiles uses slider for
> > numerical types and combines slider with calendar for some date types, I
> > believe.  Might be nice to support that at some point (maybe for
> > @appearance="full").  But that isn't trivial so we are doing spinbuttons first.
> >  To do that we'd probably have to refactor datepicker to some degree.
> > 
> 
> Ok, in general spin widget looks good for range since spin supports
> start/end/step. But I'd propose to use for example appearance="minimal" for
> spin presentation and don't do spin presentation available by default. By
> default I think it's better to use slider control, it will be go with specs.
> Then we should stop spin control using for xforms input controls. What do you
> think?
> 

I think that spin button needs to be the default until slider becomes capable of representing data types other than numerical data types.  When that is the case, then I don't mind if spin buttons aren't the default.
Comment 35 Merle Sterling 2006-10-02 12:37:55 PDT
(In reply to comment #31)
> Merle, do you think about xul controls support? I belive your bindings can be
> splitted on two, one is base bingind, other is xhtml binding. Probably in this case you don't need in huge amount of new files.
You would have even more new files. I didn't want to put all of the new bindings in one file because the file would be huge, so I split them into what I consider logical groupings; eg all of the gregorian calendar types are in one file - range-gregorian.

There is some opportunity for new base classes but you would need one new base for each of the 10 new types as there isn't common functionality that applies to all of them. I think that is best done in a follow-on bug as it won't affect the functionality.  I'd like to get this patch and the associated patch from 331987 checked-in quickly and then we can improve it from there.

> >+          this.monthSpin = document.getAnonymousElementByAttribute(this, "anonid", "monthSpin");
> >+          this.daySpin = document.getAnonymousElementByAttribute(this, "anonid", "daySpin");
> >+          this.yearSpin = document.getAnonymousElementByAttribute(this, "anonid", "yearSpin");
> That's not usual practice. Use the next template:
> <property name="monthSpin" readonly="true">
>   <getter>
>     if (!this._monthSpin) {
>       this._monthSpin = this.onwerDocument.getAnonymousElementByAttribute(this, "anonid", "monthSpin");
>     }
>     return this._monthSpin;
>   </getter>
> </property>
> <field name="_monthSpin">null</field>

Done.  I had used that pattern for the spinbutton but didn't carry it over to the new bindings.
Comment 36 aaronr 2006-10-02 13:20:32 PDT
(In reply to comment #35)
> (In reply to comment #31)
> > Merle, do you think about xul controls support? I belive your bindings can be
> > splitted on two, one is base bingind, other is xhtml binding. Probably in this case you don't need in huge amount of new files.
> You would have even more new files. I didn't want to put all of the new
> bindings in one file because the file would be huge, so I split them into what
> I consider logical groupings; eg all of the gregorian calendar types are in one
> file - range-gregorian.
> 
> There is some opportunity for new base classes but you would need one new base
> for each of the 10 new types as there isn't common functionality that applies
> to all of them. I think that is best done in a follow-on bug as it won't affect
> the functionality.  I'd like to get this patch and the associated patch from
> 331987 checked-in quickly and then we can improve it from there.
> 

I appreciate the logic of moving them into seperate files.  I really do.  But at the current pace we are going to have around 10-11 files just for range and that seems crazy, too. I don't mind a reorg being done after we get this functionality in so that we can test this work, fix any bugs and make sure that we have everything working right before we reorg.  But we've at least got to look into possibly reorging this range work even if we eventually decide that things are best left like they are to keep the files a manageable size.

Sounds like perhaps we need a base file (that holds the base binding for all of the different types as well as the range base binding), a -xhtml file for the xhtml specific stuff, and a -xul file for the xul stuff.
Comment 37 Merle Sterling 2006-10-02 14:46:42 PDT
Created attachment 240982 [details] [diff] [review]
patch
Comment 38 alexander :surkov 2006-10-02 21:17:21 PDT
Merle, this will not go for me until following things are fixed:

1) use(steal) toolkit's spinbuttons (http://lxr.mozilla.org/mozilla/source/toolkit/content/widgets/spinbuttons.xml). How they are used you can look at xul's numberbox implementation(http://lxr.mozilla.org/mozilla/source/toolkit/content/widgets/numberbox.xml). The advantages are:
  - when we will stop 1.8.x support then we'll just remove our bindings and will use toolkit's analogue.
  - spinbuttons will be grouped together, because if 'down' and 'up' buttons are on different sides of control then it's not accessible (we should look like standard spin widget)
  - spinbuttons will know nothing about parent widget, now your child calls parent methods (child widget should always avoid to call methods of parent of parent if it's possible)

2) Like Aaron said you should have three binding: base, xhtml, and xul. Look, for example, at 'range-date-xhtml'. The base methods should be 'refresh', 'parseStartDate' and so on, the content specific part should be moved into xhtml binding (for example, thease are 'monthSpin' and so on).

I vote for these two items since if you will say we will fix this later then it will involve very big code changes I guess. We'll get the case when we should be forced to refactor your widgets like I did for almost all xforms bindings.

The other issue I can see is you don't use start/end/step properties but you inherited from 'xfromswidget-range-base'. Also I guess you should implement 'getControlElement()' method for your widget. That will be compatible with other rules of xforms widgets writing.
Comment 39 alexander :surkov 2006-10-02 22:01:44 PDT
Also, please use two spaces before your name in license block. And add bindings for numeric datatypes (we should have spin controls for numeric ranges too). We can rename this bug like 'spin representation of range controls' to better reflect what you do here.

Merle, I filed bug 355198 to stop spins using for input controls. If this bug is targeted to version 0.7 then bug 355198 should be fixed there too. Merle, do you like to take that bug?
Comment 40 aaronr 2006-10-03 15:43:47 PDT
(In reply to comment #39)
> Also, please use two spaces before your name in license block. And add bindings
> for numeric datatypes (we should have spin controls for numeric ranges too). We
> can rename this bug like 'spin representation of range controls' to better
> reflect what you do here.
> 
> Merle, I filed bug 355198 to stop spins using for input controls. If this bug
> is targeted to version 0.7 then bug 355198 should be fixed there too. Merle, do
> you like to take that bug?
> 


I'd say that we NOT put this bug into 0.7.  It is going to take too long to get it in AND flush any bugs out of it.  Plus I'm sure we'll still have follow on bug work to get the UI looking/behaving right.  I'd rather it go into 0.8.
Comment 41 Merle Sterling 2006-10-13 21:50:14 PDT
Created attachment 242257 [details] [diff] [review]
patch

- Refactored new bindings based on range-base binding.
- Base binding for each new type in range.xml.
- Successor bindings for each new type in range-xhtml.
Comment 42 alexander :surkov 2006-10-14 07:50:50 PDT
Comment on attachment 242257 [details] [diff] [review]
patch


>+          this.control.monthSpin.label = "Month";
>+          this.control.daySpin.label = "Day";
>+          this.control.yearSpin.label = "Year";

Not localizable things will not go.
Comment 43 aaronr 2006-10-16 18:34:39 PDT
(In reply to comment #42)
> (From update of attachment 242257 [details] [diff] [review] [edit])
> 
> >+          this.control.monthSpin.label = "Month";
> >+          this.control.daySpin.label = "Day";
> >+          this.control.yearSpin.label = "Year";
> 
> Not localizable things will not go.
> 

one way to do this would be to use the string bundle service, just like we use in our cpp code.  An example of a place in mozilla that does this in .js would be: http://lxr.mozilla.org/mozilla/source/toolkit/content/contentAreaUtils.js#525
Comment 44 alexander :surkov 2006-10-16 19:38:04 PDT
(In reply to comment #43)
> (In reply to comment #42)
> > (From update of attachment 242257 [details] [diff] [review] [edit] [edit])
> > 
> > >+          this.control.monthSpin.label = "Month";
> > >+          this.control.daySpin.label = "Day";
> > >+          this.control.yearSpin.label = "Year";
> > 
> > Not localizable things will not go.
> > 
> 
> one way to do this would be to use the string bundle service, just like we use
> in our cpp code.  An example of a place in mozilla that does this in .js would
> be:
> http://lxr.mozilla.org/mozilla/source/toolkit/content/contentAreaUtils.js#525
> 

No. I believe we shouldn't use string bundles in xbl where it's possible. That's property of very big bindings, application level bindings. Firstly it's better to think how to reorganize bindings to use dtd. I think it's possible since year is always year and we can use static element for showing that it's year. The only one reason is dtd looks much readable than string bundles.
Comment 45 aaronr 2006-10-16 19:47:09 PDT
(In reply to comment #44)
> (In reply to comment #43)
> > (In reply to comment #42)
> > > (From update of attachment 242257 [details] [diff] [review] [edit] [edit] [edit])
> > > 
> > > >+          this.control.monthSpin.label = "Month";
> > > >+          this.control.daySpin.label = "Day";
> > > >+          this.control.yearSpin.label = "Year";
> > > 
> > > Not localizable things will not go.
> > > 
> > 
> > one way to do this would be to use the string bundle service, just like we use
> > in our cpp code.  An example of a place in mozilla that does this in .js would
> > be:
> > http://lxr.mozilla.org/mozilla/source/toolkit/content/contentAreaUtils.js#525
> > 
> 
> No. I believe we shouldn't use string bundles in xbl where it's possible.
> That's property of very big bindings, application level bindings. Firstly it's
> better to think how to reorganize bindings to use dtd. I think it's possible
> since year is always year and we can use static element for showing that it's
> year. The only one reason is dtd looks much readable than string bundles.
> 


Then we can put them in xforms.dtd and access them like datepicker does: http://lxr.mozilla.org/mozilla/source/extensions/xforms/resources/content/input-xhtml.xml#196
Comment 46 alexander :surkov 2006-10-17 05:35:10 PDT
1) I think refreshInternal() methods should be defined in range.xml.
2) Merle, probably will you try to fix bug 353880 before this one. If you will add spinbuttons widget firstly, it reduces this patch and should make things clearly. Note, even if you will not use xul's spinbuttons but you should use it's styling. Especially, -moz-appearance style.
3) It looks like widgets-xhtml are excess, probably the things will more easy if you will implement functionality of widget-xhtml bindings in range-xhtml bidings.
Comment 47 Merle Sterling 2006-10-17 09:46:29 PDT
(In reply to comment #46)
> 1) I think refreshInternal() methods should be defined in range.xml.
No. If it were possible to put refreshInternal in range.xml then its contents could be moved into refresh and refreshInternal wouldn't be necessary! refreshInternal is necessary because the bindings for the new types need to do things that the base refresh cannot do. refreshInternal is template method just like adjustRangeValues.

> 2) Merle, probably will you try to fix bug 353880 before this one. If you will add spinbuttons widget firstly, it reduces this patch and should make things clearly. Note, even if you will not use xul's spinbuttons but you should use it's styling. Especially, -moz-appearance style.
No, 353880 is the follow-on bug to improve things and should go in after this patch.  None of the methods or inheritance hieararchy will need to change to improve the spinbuttons. We need help from someone with good CSS skills to improve their appearance or we could rewrite them completely without affecting any of the other widgets for bindings.

> 3) It looks like widgets-xhtml are excess, probably the things will more easy
> if you will implement functionality of widget-xhtml bindings in range-xhtml
> bidings.
No again.  I am forced to use the getControlElement method and all of the other methds and properties in range.xml and therefore I need a widget for each type - that widget is the 'control' element. Previously they were all implemented in range-xtml bindings and were much simpler.  Now instead of 3 getAttribute calls, I have hundreds of lines of code to conform to the pattern established by the refactoring of range.xml - but that's what you wanted.

Comment 48 alexander :surkov 2006-10-17 10:09:19 PDT
(In reply to comment #47)
> (In reply to comment #46)
> > 1) I think refreshInternal() methods should be defined in range.xml.
> No. If it were possible to put refreshInternal in range.xml then its contents
> could be moved into refresh and refreshInternal wouldn't be necessary!
> refreshInternal is necessary because the bindings for the new types need to do
> things that the base refresh cannot do. refreshInternal is template method just
> like adjustRangeValues.

You add many new bindings into range.xml. I mean you should move refreshInternal() there. The reason is you will be able to reuse it for xul implementation.

> > 2) Merle, probably will you try to fix bug 353880 before this one. If you will add spinbuttons widget firstly, it reduces this patch and should make things clearly. Note, even if you will not use xul's spinbuttons but you should use it's styling. Especially, -moz-appearance style.
> No, 353880 is the follow-on bug to improve things and should go in after this
> patch.

What's the difference? You are always can replay it. My point is if this patch will easier then it will be fine.

> None of the methods or inheritance hieararchy will need to change to
> improve the spinbuttons. We need help from someone with good CSS skills to
> improve their appearance or we could rewrite them completely without affecting
> any of the other widgets for bindings.

There are styles in toolkit. Why do we need something special? Do they not work for us?

> > 3) It looks like widgets-xhtml are excess, probably the things will more easy
> > if you will implement functionality of widget-xhtml bindings in range-xhtml
> > bidings.
> No again.  I am forced to use the getControlElement method and all of the other
> methds and properties in range.xml and therefore I need a widget for each type
> - that widget is the 'control' element. Previously they were all implemented in
> range-xtml bindings and were much simpler.  Now instead of 3 getAttribute
> calls, I have hundreds of lines of code to conform to the pattern established
> by the refactoring of range.xml - but that's what you wanted.
> 

Merle, my english is not so clear and I think I'm not always able to say you what I actually mean. I don't like as you these hundreds of line of code. But I didn't remember I said that you should split your bindings on three. I think we should have bindings spinbuttons and bindings for xhtml ranges. I'm not sure you need in huge amount of bindings in widgets-xhtml.xml.
Comment 49 aaronr 2006-10-26 16:24:33 PDT
Comment on attachment 242257 [details] [diff] [review]
patch

removing from review queue.  New version will be coming with reorg of the bindings.
Comment 50 alexander :surkov 2007-01-28 03:56:37 PST
Merle, you can steal idea or even code from bug 92174.
Comment 51 alexander :surkov 2007-01-29 10:47:55 PST
Merle, I'd like if your native widgets for range will adress requirements of bug 365907 too.

IIRC bug 92174 suggests the widget that contains text fields for day, month, year and it looks fine. For range I'd suggested to have widget with those three fields with one spinbuttons controls. Spinbuttons control increment/decrement the focused (or last previously focused) field. But for calendar control I need to have widget  that contains month and year fields with spinbuttons controls for every field. So, I think to achive this you should have interface widget (widgets.xml) that will provide increment/decrement methods and at least two content widgets (widgets-xul.xml) (one is for calendar, other(s) is for range. What do you think?
Comment 52 Merle Sterling 2007-01-31 18:14:38 PST
Alex, I believe the interface I am currently working on will work for you. I sent you an email describing what I am doing to make sure we are on the same page.  Did you happen to change your email address?
Comment 53 alexander :surkov 2007-02-01 04:38:07 PST
(In reply to comment #52)
> Alex, I believe the interface I am currently working on will work for you. I
> sent you an email describing what I am doing to make sure we are on the same
> page.  Did you happen to change your email address?
> 

I sent an email to you. But it sounds you did't receive it. I guess IBM banned my mail box :).

Please look at the bug 365907. I use monthyear widget there. Please see is it possible to share code between your widgets and that one.
Comment 54 Merle Sterling 2007-02-02 13:45:27 PST
Created attachment 253785 [details] [diff] [review]
dateTime range

This patch is for a range of type dateTime.
- Interface for range widgets that use spinbuttons to represent the components of a type (widgets.xml).
- Interface for date and dateTime widgets (widgets.xml)
- Content widget for dateTime (widgets-xhtml.xml)
- range binding for dateTime ranges (range-xhtml.xml)

The 'setSpinbuttonMinMax' method in the range-dateTime widget is rather long and complicated to deal with all of the possible valid date ranges and will be identical for a date range without a time. I need to break this method down into individual setYearMin/Max, setMonthMin/Max, setDayMin/Max etc methods and put them in the date/dateTime interface widget so that xhtml date ranges and non-xhtml controls could use them too without having to duplicate all that code.
Comment 55 alexander :surkov 2007-02-06 08:10:33 PST
Comment on attachment 253785 [details] [diff] [review]
dateTime range

It looks that your patch has all to add simply range control for XUL. Please finish XUL support to have complete idea of your approach.


>+  <!-- DATETIME RANGE-->
>+  <binding id="range-datetime"
>+           extends="chrome://xforms/content/widgets.xml#datetime">
>+  
>+    <content>
>+      <children includes="label"/>
>+      <html:table>
>+        <html:body>
>+          <html:tr>
>+            <html:td>
>+              <xul:hbox>
>+                <xul:label value="&xforms.date.year.label;"/>
>+                <xul:textbox type="number" anonid="yearSpin"/>
>+              </xul:hbox>

Also, I'd like if range controls will be compatible with analogies toolkit's widgets.

Documentation is at http://wiki.mozilla.org/XUL:Specs:DateTimePickers
Tests and examples are at http://xulplanet.com/ndeakin/tests/xts/dtpicker/

Do we like this?
Comment 56 alexander :surkov 2007-02-06 08:12:34 PST
Comment on attachment 253785 [details] [diff] [review]
dateTime range

I cancel review because I hope to have the patch with XUL documents support :) Merle, does it work for you?
Comment 57 Steve Speicher 2007-02-06 09:43:11 PST
I'd recommend moving forward with XHTML and address XUL in separate bug.  This bug keeps growing.
Comment 58 Merle Sterling 2007-02-06 10:39:26 PST
(In reply to comment #55)
> (From update of attachment 253785 [details] [diff] [review])
> It looks that your patch has all to add simply range control for XUL. Please
> finish XUL support to have complete idea of your approach.
I also think that xul controls should be done later, if ever, in a separate bug. I believe the interface is sufficient to do non-xhtml controls at a future point but that is not the focus of this bug. 

> >+  <!-- DATETIME RANGE-->
> >+  <binding id="range-datetime"
> >+           extends="chrome://xforms/content/widgets.xml#datetime">
> >+  
> >+    <content>
> >+      <children includes="label"/>
> >+      <html:table>
> >+        <html:body>
> >+          <html:tr>
> >+            <html:td>
> >+              <xul:hbox>
> >+                <xul:label value="&xforms.date.year.label;"/>
> >+                <xul:textbox type="number" anonid="yearSpin"/>
> >+              </xul:hbox>
> Also, I'd like if range controls will be compatible with analogies toolkit's
> widgets.
> Documentation is at http://wiki.mozilla.org/XUL:Specs:DateTimePickers
> Tests and examples are at http://xulplanet.com/ndeakin/tests/xts/dtpicker/
> Do we like this?

If the idea is to change from individual spinbuttons for the components of a date to multiple textboxes with one spinbutton, then no we do not like that idea. We've discussed that idea before. We need to stay on track and move forward with the current approach and get it done.
Comment 59 alexander :surkov 2007-02-06 11:46:24 PST
(In reply to comment #58)
> (In reply to comment #55)
> > (From update of attachment 253785 [details] [diff] [review] [details])
> > It looks that your patch has all to add simply range control for XUL. Please
> > finish XUL support to have complete idea of your approach.

> I also think that xul controls should be done later, if ever, in a separate
> bug. I believe the interface is sufficient to do non-xhtml controls at a future
> point but that is not the focus of this bug. 

Really XUL support won't grow your patch but it may change your binding a little. Therefore I'd very like to see XUL, it will helps a lot in understanding of your approach. Merle, I believe it wouldn't take a lot of your time because it looks your patch has anything to keep XUL excepting some CSS styles.
Comment 60 alexander :surkov 2007-02-06 11:47:53 PST
(In reply to comment #58)

> > Also, I'd like if range controls will be compatible with analogies toolkit's
> > widgets.
> > Documentation is at http://wiki.mozilla.org/XUL:Specs:DateTimePickers
> > Tests and examples are at http://xulplanet.com/ndeakin/tests/xts/dtpicker/
> > Do we like this?
> 
> If the idea is to change from individual spinbuttons for the components of a
> date to multiple textboxes with one spinbutton, then no we do not like that
> idea. We've discussed that idea before. We need to stay on track and move
> forward with the current approach and get it done.
> 

I'm fine with this if it works for Aaron and Olli.
Comment 61 aaronr 2007-02-06 12:23:16 PST
(In reply to comment #59)
> (In reply to comment #58)
> > (In reply to comment #55)
> > > (From update of attachment 253785 [details] [diff] [review] [details] [details])
> > > It looks that your patch has all to add simply range control for XUL. Please
> > > finish XUL support to have complete idea of your approach.
> 
> > I also think that xul controls should be done later, if ever, in a separate
> > bug. I believe the interface is sufficient to do non-xhtml controls at a future
> > point but that is not the focus of this bug. 
> 
> Really XUL support won't grow your patch but it may change your binding a
> little. Therefore I'd very like to see XUL, it will helps a lot in
> understanding of your approach. Merle, I believe it wouldn't take a lot of your
> time because it looks your patch has anything to keep XUL excepting some CSS
> styles.
> 

I agree with Alex on this one.  Supporting XUL version of the range isn't an option.  We've got to do it at some point.  Now is as good a time as any.
Comment 62 aaronr 2007-02-06 12:24:36 PST
(In reply to comment #60)
> (In reply to comment #58)
> 
> > > Also, I'd like if range controls will be compatible with analogies toolkit's
> > > widgets.
> > > Documentation is at http://wiki.mozilla.org/XUL:Specs:DateTimePickers
> > > Tests and examples are at http://xulplanet.com/ndeakin/tests/xts/dtpicker/
> > > Do we like this?
> > 
> > If the idea is to change from individual spinbuttons for the components of a
> > date to multiple textboxes with one spinbutton, then no we do not like that
> > idea. We've discussed that idea before. We need to stay on track and move
> > forward with the current approach and get it done.
> > 
> 
> I'm fine with this if it works for Aaron and Olli.
> 

I agree with Merle on this one.  We've been working on this feature since October.  We've got to get it out there for people to use and to comment on.
Comment 63 Merle Sterling 2007-02-15 14:25:26 PST
Comment on attachment 253785 [details] [diff] [review]
dateTime range

Created bug 370551 for dateTime range. Current patch and xhtml/xul testcaes are attached there.
Comment 64 Merle Sterling 2007-02-22 12:07:22 PST
Investigate whether separate increment/decrement handlers can be consolidated into one handler. If a single handler will work for all range types (as it does for dateTime) we could simplify the code a bit.
Comment 65 Merle Sterling 2007-04-16 11:16:26 PDT
Bugs 372736, 372737, and 372739 have been fixed, so this meta bug is fixed.

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