Closed Bug 271044 Opened 20 years ago Closed 19 years ago

Implement xforms:range element

Categories

(Core Graveyard :: XForms, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: robert, Assigned: allan)

References

()

Details

(Keywords: fixed1.8.0.2, fixed1.8.1)

Attachments

(1 file, 8 obsolete files)

User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5) Gecko/20041111 Firefox/1.0 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5) Gecko/20041111 Firefox/1.0 Implement the xforms range element Reproducible: Always Steps to Reproduce:
Attached patch Initial implementation of the element (obsolete) (deleted) — Splinter Review
This implementation uses an XUL scrollbar element. currently it only supports positive integer values from the model. This ui element needs to be tigthly integrated with the schema of the value being edited, in order to get the min, max and step value. For non positive integer values, the idea is to scale the values to generate the required min, max and step size because it only accept integer values. How can i get the schema type of a node, if it has range restrictions? Is that part implemented?
I don't think that there is any function written, yet, to easily determine the type of a node. But the information is there. The MDG has that information, and also I think that you can get the MIP's (model item properties, of which "type" is one) off of the instance node that you bind to. Looks like in nsXFormsModelElement::ProcessBind it sets the MIP's as properties off of each node of the nodeset that the bind applies to. Also in ProcessBind, it sets the MIP's in the MDG for each node. For your case, I think that you should forget the MDG approach, because that won't have "types" of instance nodes that aren't part of a binding (i.e. instance nodes that have the "type" attribute right on them). I think it would be a better approcach for you to get the instance node that the range is single-node bound to and get the type directly from it.
Assignee: aaronr → robert
The MDG does not care much about the type. IMHO, the schema support that Doron is working on, is the place to get that information.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
I managed to get this to build on trunk.
[I'm guessing that Robert is going nowhere with this, reassigning to bug owner.] My thoughts are that it should be done in XBL, and we need to define an interface exposed by the XForms engine, something with at least: readonly attribute integer min readonly attribute integer max readonly attribute integer step (though min and max can also be unbound also.) Note that the spec. says: "In the event of overlapping restrictions between the underlying datatype and the start and end hints, the most restrictive range should be used." (http://www.w3.org/TR/xforms/slice8.html#ui-range) For step, there's no specification, but something like "max-min/20" and then restricted/converted to the type (ie. if days, step shouldn't be "1.3 day").
Assignee: robert → aaronr
Status: ASSIGNED → NEW
Attached patch _rough_ XBL version of range (obsolete) (deleted) — Splinter Review
Here's a go at an XBL version of range. It's crappy crap, it just shows the interface, etc.. Only shows the bound value, does not set it (events disappear somewhere with an assertion about not being in a XUL document).
Attached patch Patch using canvas (obsolete) (deleted) — Splinter Review
Works ok for me. Needs some more refining though, especially support for in/out-of-range. There's a TODO in range.xml
Attachment #189892 - Attachment is obsolete: true
Status update: I have it supporting out of range values, and dispatches the corresponding events, etc., so it actually passes two of the test suite tests now. I still need to handle: * handle @incremental="true" * handle undefined step * handle undefined begin / end Undefined step is fairly easy I think. But how about undefined begin and/or end? If both are undefined we should probably use a "knob control" instead, but what about only one of them?
Are some files missing from the canvas-using patch? I don't see any canvas usage..
(In reply to comment #9) > Are some files missing from the canvas-using patch? I don't see any canvas usage.. What the? It's a totally out-dated patch I've uploaded. Argh. I'll upload a correct version asap.
Attached patch Patch using canvas (the correct patch...) (obsolete) (deleted) — Splinter Review
Attachment #192729 - Attachment is obsolete: true
hmm, I wasn't CC'd.
Allan, what is still missing here? The patch doesn't apply cleanly to trunk anymore.
(In reply to comment #13) > Allan, what is still missing here? 1. Acknowledgement on that we want to use canvas 2. Handle ranges without start or end. I would go for a followup bug, which implements a "knob" control or something like that. > The patch doesn't apply cleanly to trunk anymore. Merde. I'll get to this, my request queue, etc. after tomorrow.
Attached patch Updated patch (obsolete) (deleted) — Splinter Review
There are still todo's, which are listed in the header of resources/content/range.xml.
Assignee: aaronr → allan
Attachment #193029 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #200057 - Flags: review?(aaronr)
Comment on attachment 200057 [details] [diff] [review] Updated patch >--- xforms/nsXFormsRangeElement.cpp 1970-01-01 01:00:00.000000000 +0100 >+++ xforms.range/nsXFormsRangeElement.cpp 2005-10-19 09:50:05.000000000 +0200 >+// nsXFormsRangeElement >+nsresult >+nsXFormsRangeElement::AttributeGetter(const nsAString &aAttr, nsAString &aVal) >+{ >+ PRBool hasAttr; >+ nsAutoString val; >+ if (mElement >+ && NS_SUCCEEDED(mElement->HasAttribute(aAttr, &hasAttr)) >+ && hasAttr) { >+ mElement->GetAttribute(aAttr, val); >+ } >+ if (val.IsEmpty()) { >+ SetDOMStringToNull(aVal); >+ } else { >+ aVal = val; >+ } >+ >+ return NS_OK; >+} >+ >+// nsIXFormsRangeElement >+NS_IMETHODIMP >+nsXFormsRangeElement::GetRangeStart(nsAString &aMin) >+{ >+ return AttributeGetter(NS_LITERAL_STRING("start"), aMin); >+} >+ >+NS_IMETHODIMP >+nsXFormsRangeElement::GetRangeEnd(nsAString &aMax) >+{ >+ return AttributeGetter(NS_LITERAL_STRING("end"), aMax); >+} >+ >+NS_IMETHODIMP >+nsXFormsRangeElement::GetRangeStep(nsAString &aStep) >+{ >+ return AttributeGetter(NS_LITERAL_STRING("step"), aStep); >+} Can't you do this.getAttribute in JS for this? Or put AttributeGetter on delegate and do the others in JS? Nothing range specific in here and could be used by others. >+ <method name="calcPos"> >+ <parameter name="val"/> >+ <body> >+ var pos = val - this.rBegin; >+ if (this.rStep) { >+ pos = (pos / this.rStep) * this.stepsp; >+ } else { >+ pos = (pos / (this.rEnd - this.rBegin)) * this.barwidth; >+ } >+ return Math.round(pos) + this.margin; >+ </body> >+ </method> comment, please. >+ >+ <!-- sets the slider to a new value --> >+ <method name="setSlider"> >+ <!-- The new value --> >+ <parameter name="aVal"/> >+ <!-- The mode: move, set --> >+ <parameter name="aMode"/> Comment, please, what it means to setSlider if aMode = "set" versus "move". Or if there is no aMode. >+ >+ <body> >+ <![CDATA[ >+ >+ aVal = parseFloat(aVal); >+ if (aMode != "set" && isNaN(aVal)) { >+ return alert("NaN passed to setSlider()!"); >+ } >+ >+ var outOfRange = false; nit: alignment. Some all through this file. >+ >+ // clear old slider >+ this.ctx.clearRect(this.calcPos(this.rVal) - this.sliderwidth - 1, this.tickheight - 1, >+ this.sliderwidth * 2 + 2, this.tickheight * 3 + 2); >+ >+ // (re)draw horisontal bar >+ this.ctx.lineWidth = 1; >+ this.ctx.fillStyle = this.fillStyle; >+ this.ctx.strokeStyle = this.strokeStyle; >+ mid = Math.round(this.height / 2); >+ // XXX only needs to be redrawn for old slider pos >+ this.ctx.fillRect(this.margin, mid - 1, this.barwidth, 3); >+ this.ctx.strokeRect(this.margin, mid - 1, this.barwidth, 3); >+ >+ // check whether out-of-range state has changed, and dispatch event if it has >+ if (outOfRange != this.outOfRange) { >+ this.outOfRange = outOfRange; >+ var event = outOfRange ? "xforms-out-of-range" : "xforms-in-range"; >+ this.delegate.dispatchXFormsNotificationEvent(event, this); >+ } >+ >+ // if out-of-range, we cannot represent the value >+ if (outOfRange) { >+ this.rVal = null; >+ return null; >+ } Need to style it as out-of-range too? >+ <!-- create new range object --> >+ <method name="createRange"> >+ <parameter name="aCanvas"/> >+ <parameter name="aLabelBegin"/> >+ <parameter name="aLabelEnd"/> >+ <parameter name="aBegin"/> >+ <parameter name="aEnd"/> >+ <parameter name="aStep"/> >+ <body> >+ <![CDATA[ >+ if (!(aCanvas && aLabelBegin && aLabelEnd)) { >+ alert("One or more of the passed objects were null"); >+ return false; >+ } >+ >+ this.rBegin = parseFloat(aBegin); >+ this.rEnd = parseFloat(aEnd); >+ this.rStep = parseFloat(aStep); >+ this.rVal = this.rBegin; >+ this.isIncremental = this.getAttribute("incremental") == "true"; >+ this.justMoved = false; >+ >+ if (isNaN(this.rBegin) || isNaN(this.rEnd)) { >+ alert("One or more init() parameters is NaN!"); >+ return false; >+ } >+ >+ // XXX should we handle this? >+ if (this.rBegin >= this.rEnd) { >+ alert("begin is higher than end?"); >+ return false; >+ } Ummm, alerts? These are temporary, I assume, since start and end are strings, not numbers. > >diff -X patch-excludes -uprN -U8 xforms/resources/content/xforms.xml xforms.range/resources/content/xforms.xml >--- xforms/resources/content/xforms.xml 2005-10-19 09:49:14.000000000 +0200 >+++ xforms.range/resources/content/xforms.xml 2005-10-19 09:56:29.000000000 +0200 >@@ -99,24 +99,41 @@ > </method> > > <method name="focus"> > <body> > return false; > </body> > </method> > >+ <!-- Dispatch DOMActivate to the control itself --> > <method name="dispatchDOMActivate"> > <body> > var ev = document.createEvent("UIEvents"); > ev.initUIEvent("DOMActivate", true, true, window, 1); > this.dispatchEvent(ev); > return true; > </body> > </method> >+ >+ <!-- >+ Dispatch an xforms notification event to a node. >+ >+ See http://www.w3.org/TR/xforms/slice4.html#evt-notify >+ --> >+ <method name="dispatchXFormsNotificationEvent"> >+ <parameter name="aEventName"/> >+ <parameter name="aTarget"/> >+ <body> >+ var ev = document.createEvent("Events"); >+ ev.initEvent(aEventName, true, false); >+ aTarget.dispatchEvent(ev); >+ return true; >+ </body> >+ </method> > </implementation> > </binding> You might want to add comment here as to what messages this will work for and which it won't. Can't really predict who will use this for what, so might want to at least give them a heads up that bubbles="true" and cancelable="false" and these aren't necessarily the values for all XForms events. Maybe what we should really have is a way to call nsXFormsUtils::DispatchEvent from JS so that the bubble and cancelable events are set up automatically. This is a good stab at a beginning for range. Good to get this patch in before it grows much more. I didn't make any comments in here that I really need to follow up on, I don't think. so r=me with my comments addressed.
Attachment #200057 - Flags: review?(aaronr) → review+
(In reply to comment #16) > (From update of attachment 200057 [details] [diff] [review] [edit]) > >+NS_IMETHODIMP > >+nsXFormsRangeElement::GetRangeStart(nsAString &aMin) > >+{ > >+ return AttributeGetter(NS_LITERAL_STRING("start"), aMin); > >+} > >+ > >+NS_IMETHODIMP > >+nsXFormsRangeElement::GetRangeEnd(nsAString &aMax) > >+{ > >+ return AttributeGetter(NS_LITERAL_STRING("end"), aMax); > >+} > >+ > >+NS_IMETHODIMP > >+nsXFormsRangeElement::GetRangeStep(nsAString &aStep) > >+{ > >+ return AttributeGetter(NS_LITERAL_STRING("step"), aStep); > >+} > > Can't you do this.getAttribute in JS for this? Or put AttributeGetter > on delegate and do the others in JS? Nothing range specific in here > and could be used by others. Yeah, they look quite stupid. I had a todo in the start of the file, I've moved it down here to the functions. The XTF range needs to check the type of the instance data node and then for GetRangeStart() it needs to return "max(type.minimum, @start)", and vice versa for GetRangeEnd() For the step, it needs to set the step to something "start" which also accounts for the type, if @step is not set. > nit: alignment. Some all through this file. > >+ // check whether out-of-range state has changed, and dispatch event if it has > >+ if (outOfRange != this.outOfRange) { > >+ this.outOfRange = outOfRange; > >+ var event = outOfRange ? "xforms-out-of-range" : "xforms-in-range"; > >+ this.delegate.dispatchXFormsNotificationEvent(event, this); > >+ } > >+ > >+ // if out-of-range, we cannot represent the value > >+ if (outOfRange) { > >+ this.rVal = null; > >+ return null; > >+ } > > Need to style it as out-of-range too? Hmm, yes, good catch. This means more functionality on the XTF interface... hmm. If it's ok, I'll follow up on that. > > >+ <!-- create new range object --> > >+ <method name="createRange"> > >+ <parameter name="aCanvas"/> > >+ <parameter name="aLabelBegin"/> > >+ <parameter name="aLabelEnd"/> > >+ <parameter name="aBegin"/> > >+ <parameter name="aEnd"/> > >+ <parameter name="aStep"/> > >+ <body> > >+ <![CDATA[ > >+ if (!(aCanvas && aLabelBegin && aLabelEnd)) { > >+ alert("One or more of the passed objects were null"); > >+ return false; > >+ } > >+ > >+ this.rBegin = parseFloat(aBegin); > >+ this.rEnd = parseFloat(aEnd); > >+ this.rStep = parseFloat(aStep); > >+ this.rVal = this.rBegin; > >+ this.isIncremental = this.getAttribute("incremental") == "true"; > >+ this.justMoved = false; > >+ > >+ if (isNaN(this.rBegin) || isNaN(this.rEnd)) { > >+ alert("One or more init() parameters is NaN!"); > >+ return false; > >+ } > >+ > >+ // XXX should we handle this? > >+ if (this.rBegin >= this.rEnd) { > >+ alert("begin is higher than end?"); > >+ return false; > >+ } > > Ummm, alerts? These are temporary, I assume, since start and end are > strings, not numbers. I only support floats for now, so they are numbers. An alert is wrong though. I'll emit a scripterror instead. > >+ > >+ <!-- > >+ Dispatch an xforms notification event to a node. > >+ > >+ See http://www.w3.org/TR/xforms/slice4.html#evt-notify > >+ --> > >+ <method name="dispatchXFormsNotificationEvent"> > >+ <parameter name="aEventName"/> > >+ <parameter name="aTarget"/> > >+ <body> > >+ var ev = document.createEvent("Events"); > >+ ev.initEvent(aEventName, true, false); > >+ aTarget.dispatchEvent(ev); > >+ return true; > >+ </body> > >+ </method> > > </implementation> > > </binding> > > You might want to add comment here as to what messages this will work for > and which it won't. Can't really predict who will use this for what, so > might want to at least give them a heads up that bubbles="true" and > cancelable="false" and these aren't necessarily the values for all XForms > events. Maybe what we should really have is a way to call > nsXFormsUtils::DispatchEvent from JS so that the bubble and cancelable > events are set up automatically. Ok, admittedly, I'm being a bit sneaky here, but I explicitly write "xforms notification event", which all have cancelable="false" and bubbles="true". I've underlined it, dunno if that helps.
Attached patch Commens fixed and exposing ReportError() (obsolete) (deleted) — Splinter Review
I fixed your comments and exposed nsXFormsUtils::ReportError(), you better ok the last one.
Attachment #200057 - Attachment is obsolete: true
Attachment #200345 - Flags: review?(aaronr)
Note to myself: the css rule should be more specific, it should be something like range { -moz-binding: url('chrome://xforms/content/xforms.xml#xformswidget-input'); } range[mozType|type="http://www.w3.org/2001/XMLSchema#decimal"] { -moz-binding: url('chrome://xforms/content/xforms.xml#xformswidget-range'); } so the actual range control is only bound to to the datatypes that it support. Instead of input we could have a "xformswidget-disabled-control". Dunno, how it should look though.
(In reply to comment #19) > Note to myself: the css rule should be more specific, it should be something like > range { > -moz-binding: url('chrome://xforms/content/xforms.xml#xformswidget-input'); > } > > range[mozType|type="http://www.w3.org/2001/XMLSchema#decimal"] { > -moz-binding: url('chrome://xforms/content/xforms.xml#xformswidget-range'); > } > > so the actual range control is only bound to to the datatypes that it support. > Instead of input we could have a "xformswidget-disabled-control". Dunno, how it > should look though. I'd say if it isn't bound to a supported node that you output a warning to the JS console and treat it like upload... gray out the range and also 'disable' it.
Comment on attachment 200345 [details] [diff] [review] Commens fixed and exposing ReportError() Excellent! I LOVE having ReportError exposed. Will make debugging events (esp. focus events) much easier from JS when you only care about the sequence of things and don't want to dismiss a bunch of alerts.
Attachment #200345 - Flags: review?(aaronr) → review+
Attachment #200345 - Flags: review?(smaug)
(In reply to comment #21) > (From update of attachment 200345 [details] [diff] [review] [edit]) > Excellent! I LOVE having ReportError exposed. Will make debugging events > (esp. focus events) much easier from JS when you only care about the sequence > of things and don't want to dismiss a bunch of alerts. Beware though that it only handles strings from bundles. We need to tweak ReportError to allow for arbitrary strings, which would be very good in a Custom Control world.
Comment on attachment 200345 [details] [diff] [review] Commens fixed and exposing ReportError() >+class nsXFormsRangeElement : public nsXFormsDelegateStub, >+ public nsIXFormsRangeElement >+{ >+private: >+ /// @note (XXX) use NS_FORWARD to make range inherit delegate >+ /// >+ // NS_FORWARD_NSIDOMELEMENT(nsGenericHTMLElement::) Could you explain this? >+NS_IMETHODIMP >+nsXFormsRangeElement::SetValue(const nsAString& aValue) >+{ >+ if (!mBoundNode || !mModel) >+ return NS_OK; >+ >+ PRBool changed; >+ nsresult rv = mModel->SetNodeValue(mBoundNode, aValue, &changed); >+ NS_ENSURE_SUCCESS(rv, rv); >+ if (changed) { >+ nsCOMPtr<nsIDOMNode> model = do_QueryInterface(mModel); >+ >+ if (model) { >+ rv = nsXFormsUtils::DispatchEvent(model, eEvent_Recalculate); >+ NS_ENSURE_SUCCESS(rv, rv); >+ rv = nsXFormsUtils::DispatchEvent(model, eEvent_Revalidate); >+ NS_ENSURE_SUCCESS(rv, rv); >+ rv = nsXFormsUtils::DispatchEvent(model, eEvent_Refresh); >+ NS_ENSURE_SUCCESS(rv, rv); >+ } >+ } >+ >+ return NS_OK; >+} This method is copy-pasted from nsXFormsDelegateStub. Why? Please remove, if it is not needed. >+ >+// nsXFormsRangeElement >+nsresult >+nsXFormsRangeElement::AttributeGetter(const nsAString &aAttr, nsAString &aVal) >+{ >+ PRBool hasAttr; >+ nsAutoString val; >+ if (mElement >+ && NS_SUCCEEDED(mElement->HasAttribute(aAttr, &hasAttr)) >+ && hasAttr) { >+ mElement->GetAttribute(aAttr, val); >+ } >+ if (val.IsEmpty()) { >+ SetDOMStringToNull(aVal); >+ } else { >+ aVal = val; >+ } >+ >+ return NS_OK; >+} No need to use HasAttribute. GetAttribute(aVal) and if (aVal.IsEmpty()) {...} is enough. >diff -X patch-excludes -uprN -U8 xforms/resources/content/range.xml xforms.range/resources/content/range.xml >--- xforms/resources/content/range.xml 1970-01-01 01:00:00.000000000 +0100 >+++ xforms.range/resources/content/range.xml 2005-10-21 17:02:15.000000000 +0200 >@@ -0,0 +1,480 @@ >+<?xml version="1.0" encoding="utf-8"?> >+<!-- >+ ASSUMPTIONS: >+ *> @begin is valid, @end and @init value might not be >+ this means that steps and ticks are calculated with begin as starting point >+ *> Takes integers and floats >+ >+ TODO: XXX >+ *> limit amount of ticks >+ *> handle undefined begin / end >+ *> handle end < begin (including negative steps) >+ *> @incremental should round if it is bound to integer >+ >+ BUGS: XXX >+ *> leaves a trace behind, hor.bar gets darker, etc... fix transparency >+--> I think this comment should be after the Licence. With those r=me
Attachment #200345 - Flags: review?(smaug) → review+
(In reply to comment #23) > > No need to use HasAttribute. GetAttribute(aVal) and if (aVal.IsEmpty()) {...} > is enough. > Should be GetAttribute(aAttr, aVal), I guess ;)
(In reply to comment #23) > (From update of attachment 200345 [details] [diff] [review] [edit]) > >+class nsXFormsRangeElement : public nsXFormsDelegateStub, > >+ public nsIXFormsRangeElement > >+{ > >+private: > >+ /// @note (XXX) use NS_FORWARD to make range inherit delegate > >+ /// > >+ // NS_FORWARD_NSIDOMELEMENT(nsGenericHTMLElement::) > > Could you explain this? Yes, and I should have fixed it too, but aparently forgot. I wanted to make nsIXFormsRangeElement inherit from nsIXFormsDelegate, ran in to inheritance problems so I skipped that. But then I stumbled upon NS_FORWARD... noticed it, but didn't get further. I'll fix that now. (it should be NS_FORWARD_NSIXFORMSDELEGATE(nsXFormsDelegateStub::))
Attached patch Patch with smaug's comments (obsolete) (deleted) — Splinter Review
Ok, this one fixes your comments, but I think you should look at it once more, as I know let nsIXFormsRangeElement inherit from nsIXFormsDelegate. I've also fixed a problem in range.xml in setSlider(): If the bound node value was set out of range, it adjusted the value instead of just being outofrange.
Attachment #200345 - Attachment is obsolete: true
Attachment #200769 - Flags: review?(smaug)
Comment on attachment 200769 [details] [diff] [review] Patch with smaug's comments >+ >+<bindings xmlns="http://www.mozilla.org/xbl" >+ xmlns:html="http://www.w3.org/1999/xhtml"> >+ >+ <binding id="xformswidget-range" >+ extends="chrome://xforms/content/xforms.xml#xformswidget-base"> >+ <content> >+ <children includes="label"/> >+ <html:span anonid="labelBegin" style="margin-right: 3px;"></html:span> >+ <!-- width and height set by CSS? --> >+ <html:canvas tabindex="0" anonid="canvas" width="260" height="40" >+ onkeydown="this.parentNode.handleKey(event)" >+ onmousedown="this.parentNode.handleMouseDown(event)" >+ onmouseup="this.parentNode.handleMouseUp(event)" >+ onmouseout="this.parentNode.handleMouseOut(event)" >+ onmousemove="this.parentNode.handleMouseMove(event)"> >+ </html:canvas> >+ <html:span anonid="labelEnd" style="margin-left: 3px;"> </html:span> >+ <children/> >+ </content> >+ >+ <!-- handle mouse down --> >+ <method name="handleMouseDown"> >+ <parameter name="event"/> >+ <body> >+ <![CDATA[ >+ if (event.button == 0) { >+ this.currentOffset = this.getOffset(event); >+ this.originalVal = this.rVal; >+ // we've got to save the range somewhere global so the mousemove event can access it >+ document.currentrange = this; Sorry, I didn't notice this earlier, but document.currentrange looks strange. I don't quite understand why it is needed. Are you sure 'this' is not the object you need in handleMouseMove. Or if there is something wrong with that, perhaps you could add second parameter and use the right object in canvas' onmousemove.
Attachment #200769 - Flags: review?(smaug) → review-
(In reply to comment #27) > Sorry, I didn't notice this earlier, but document.currentrange looks strange. > I don't quite understand why it is needed. Are you sure 'this' is not the > object you need in handleMouseMove. Or if there is something wrong with that, > perhaps > you could add second parameter and use the right object in canvas' onmousemove. A very valid comment, I've never liked it myself. I'll investigate it.
Attached patch Patch without document.currentRange (obsolete) (deleted) — Splinter Review
(In reply to comment #27) > Sorry, I didn't notice this earlier, but document.currentrange looks strange. > I don't quite understand why it is needed. Are you sure 'this' is not the > object you need in handleMouseMove. Or if there is something wrong with that, > perhaps you could add second parameter and use the right object in canvas' > onmousemove. It was leftovers from being a cross-browser implementation... I've removed it now. Question is wheter reportError should live on the delegate or the accessor?
Attachment #166642 - Attachment is obsolete: true
Attachment #200769 - Attachment is obsolete: true
Attachment #202239 - Flags: review?(smaug)
Comment on attachment 202239 [details] [diff] [review] Patch without document.currentRange >+ >+ /** >+ * Report an error >+ * >+ * @param errorMsg The error message id >+ * >+ * @todo XXX this should be extended to allow for "raw strings", not >+ * necessarily kept in bundles. >+ */ >+ void reportError(in DOMString errorMsg); I think this can be in nsIXFormsDelegate, at least for now. So no need to change that. >+ <method name="refresh"> >+ <body> >+ <![CDATA[ >+ if (!this.isInitialized) { >+ if (!this.delegate) { >+ return; >+ } >+ var labelBegin = document.getAnonymousElementByAttribute(this, "anonid", "labelBegin"); >+ var labelEnd = document.getAnonymousElementByAttribute(this, "anonid", "labelEnd"); >+ var canvas = document.getAnonymousElementByAttribute(this, "anonid", "canvas"); >+ this.isInitialized = this.createRange(canvas, labelBegin, labelEnd, >+ this.accessors.getRangeStart(), >+ this.accessors.getRangeEnd(), >+ this.accessors.getRangeStep()); >+ } >+ >+ // XXX: does not clear range if bound node "disappears" >+ if (this.isInitialized && this.accessors.hasBoundNode()) { >+ this.setSlider(this.accessors.getValue(), "set"); >+ } >+ ]]> >+ </body> >+ </method> Remove tab before <![CDATA[ and ]]>. Also, could try to split overlong lines. With those r=me I think we can still improve this in some cases, but maybe in separate bugs. (And there are already XXX for the issues.)
Attachment #202239 - Flags: review?(smaug) → review+
Checked in to trunk
Whiteboard: xf-to-branch
Filed follow-up bugs: bug 316353, bug 316354, and bug 316355.
Attached patch Checked in version (deleted) — Splinter Review
Attachment #202239 - Attachment is obsolete: true
checked into MOZILLA_1_8_BRANCH via bug 323691. Leaving open for now until it gets into 1.8.0
Whiteboard: xf-to-branch
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Keywords: fixed1.8.0.2
Resolution: --- → FIXED
verfied fixed on MOZILLA_1_8_BRANCH
Keywords: fixed1.8.1
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: