User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050821 Firefox/1.6a1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050821 Firefox/1.6a1 There is xforms in xul example http://lab.cph.novell.com/~abeaufour/xfcalc.zip . This example redefines some xforms controls: redefined controls use xul elements instead of html elements. When I try use native xforms controls (with html elements) then such elements is not work in xul. I think the bug is dupe and actually isn't related to xforms. Allan, if you wrotes this example then please comment the bug. Why native xforms controls doesn't work with xul? Reproducible: Always

native xforms controls in xul document

For thease elements box model is broken. I watched testcase in DOM Inspector. All properties (coordinates x/y, width/height) of box model has value undefined.

It's clear. Because xforms element hasn't namespace xul or html then mozilla doesn't process it even it has binding with content. Actually problem is lying in xbl.lux

To comment #3 I filed bug 307646 .

If the bug 307646 will be fixed then the bug will not be solved. You should change styles for bindings of xforms controls for xul documents. In instance, styles for xforms input controls can be defined as: xf|input{ -moz-appearance: textfield; cursor: text; margin: 2px 4px; border: 2px solid; -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow; -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow; -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow; -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow; padding: 1px 0px 1px 2px; background-color: -moz-Field; color: -moz-FieldText; display: -moz-box; /*nota bene*/ } xf|input html|input{ margin: 0px !important; border: none !important; padding: 0px !important; background-color: inherit; color: inherit; font: inherit; -moz-box-flex: 1; /*nota bene*/ }

If there is ability to use xforms in xul then xforms controls should looks and behave self as xul controls. In instance, since xul 'textbox' element has context menu then I belive xforms 'input' elemnt should have context menu also. Threafore I guess in order to provide a work xforms controls in xul document we should rewrite some xforms controls (not styles only as I propose before).

> Allan, if you wrotes this example then please comment the bug. Why native xforms > controls doesn't work with xul? As you have found out there are more than one issue. But for "XForms in XUL out of the box", we would need to: 1) create implementations for all XForms controls in XUL instead of using HTML components. 2) figure out how to do bindings that the depend on the namespace of the document. Is there a "root node" target in CSS?

(In reply to comment #7) > 2) figure out how to do bindings that the depend on the namespace of the > document. Is there a "root node" target in CSS? Yes, :root pseudo-class is working. > 1) create implementations for all XForms controls in XUL instead of using HTML > components. I can try to create implementation at least partly. I should use XForms in XUL and threafore I will should realize XForms controls in XUL in any case.

(In reply to comment #8) > (In reply to comment #7) > > > 2) figure out how to do bindings that the depend on the namespace of the > > document. Is there a "root node" target in CSS? > > Yes, :root pseudo-class is working. Ok. So we could change our current bindings from "xf|input" to "xhtml|:root xf|input", and you could do "xul|:root xf|input"? > > 1) create implementations for all XForms controls in XUL instead of using HTML > > components. > > I can try to create implementation at least partly. I should use XForms in XUL > and threafore I will should realize XForms controls in XUL in any case. Most of the elements will be simple, and you can inherit from the common implementation, but implementing select and select1 will be tricky. But give it a go.

(In reply to comment #9) > Most of the elements will be simple, and you can inherit from the common > implementation, but implementing select and select1 will be tricky. But give it > a go. I propose to mark out content from interface for current bindings of xfroms controls. It has one advantage. In this case we should be able to use one interface for two contents: binding for xul and binding for xhtml. In instance: <binding id="input-iface"> <implementation> <!-- here behaviour of xforms:input --> </implementation> </binding> <binding id="html-input" extends="#input-iface"> <content> <!-- here content for xhtml --> </content> <binding id="xul-input" extends="#input-iface"> <content> <!-- here content for xul --> </content> What do you think about it? I guess this bug should be depended on bug 306764, since fixing of it involves a changes of bindings for xforms controls.

(In reply to comment #10) > (In reply to comment #9) > > > Most of the elements will be simple, and you can inherit from the common > > implementation, but implementing select and select1 will be tricky. But give it > > a go. > > I propose to mark out content from interface for current bindings of xfroms > controls. It has one advantage. In this case we should be able to use one > interface for two contents: binding for xul and binding for xhtml. > > In instance: > > <binding id="input-iface"> > <implementation> > <!-- here behaviour of xforms:input --> > </implementation> > </binding> > > <binding id="html-input" extends="#input-iface"> > <content> > <!-- here content for xhtml --> > </content> > > <binding id="xul-input" extends="#input-iface"> > <content> > <!-- here content for xul --> > </content> > > What do you think about it? If it's possible in practice to do that seperation, then it's fine with me, but I fear than you cannot do as clean a cut as that. Let's see. > I guess this bug should be depended on bug 306764, since fixing of it > involves a changes of bindings for xforms controls. Possibly, yes.

Test case for xhtml controls (don't including select and select1 controls)

Test case for xul controls (don't including select and select1 controls)

Comment on attachment 195370 [details] testcase ><window width="600" height="230" title="XForms Calculator" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:xforms="http://www.w3.org/2002/xforms"><xforms:model id="xf_model"> > <xforms:instance> > <equation xmlns=""> > <screen>0</screen> > <screenbuffer>0</screenbuffer> > <first>0</first> > <second>0</second> > <memory>0</memory> > <result/> > </equation> > </xforms:instance> > </xforms:model><vbox><hbox style="font-size: 30pt;"><spacer flex="1"/><xforms:output ref="/equation/screen"/><spacer flex="1"/></hbox><hbox><xforms:output ref="/equation/memory"> > <xforms:label>M:</xforms:label> > </xforms:output></hbox><hbox><xforms:trigger> > <xforms:label>MC</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/memory" value="0"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>7</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10 + 7"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>8</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10 + 8"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>9</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10 + 9"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>/</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/first" value="/equation/screen"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > <xforms:toggle ev:event="DOMActivate" case="divide"/> > </xforms:action> > </xforms:trigger></hbox><hbox><xforms:trigger> > <xforms:label>MR</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/memory"/> > <xforms:setvalue ref="/equation/screen" value="/equation/memory"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>4</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10 + 4"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>5</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10 + 5"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>6</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10 + 6"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>*</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/first" value="/equation/screen"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > <xforms:toggle ev:event="DOMActivate" case="multiply"/> > </xforms:action> > </xforms:trigger></hbox><hbox><xforms:trigger> > <xforms:label>MS</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/memory" value="/equation/screen"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>1</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10 + 1"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>2</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10 + 2"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>3</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10 + 3"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>-</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/first" value="/equation/screen"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > <xforms:toggle ev:event="DOMActivate" case="subtract"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>1/x</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screen" value="1 div /equation/screen"/> > </xforms:action> > </xforms:trigger></hbox><hbox><xforms:trigger> > <xforms:label>M+</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/memory" value="/equation/memory + /equation/screen"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>0</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screenbuffer" value="/equation/screenbuffer * 10"/> > <xforms:setvalue ref="/equation/screen" value="/equation/screenbuffer"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>+/-</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/screen" value="/equation/screen * -1"/> > </xforms:action> > </xforms:trigger><xforms:trigger> > <xforms:label>+</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/first" value="/equation/screen"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > <xforms:toggle ev:event="DOMActivate" case="add"/> > </xforms:action> > </xforms:trigger><xforms:switch> > <xforms:case id="add" selected="true"> > <xforms:trigger> > <xforms:label>=</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/second" value="/equation/screenbuffer"/> > <xforms:setvalue ref="/equation/result" value="/equation/first + /equation/second"/> > <xforms:setvalue ref="/equation/screen" value="/equation/result"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > </xforms:action> > </xforms:trigger> > </xforms:case> > <xforms:case id="subtract"> > <xforms:trigger> > <xforms:label>=</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/second" value="/equation/screenbuffer"/> > <xforms:setvalue ref="/equation/result" value="/equation/first - /equation/second"/> > <xforms:setvalue ref="/equation/screen" value="/equation/result"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > </xforms:action> > </xforms:trigger> > </xforms:case> > <xforms:case id="multiply"> > <xforms:trigger> > <xforms:label>=</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/second" value="/equation/screenbuffer"/> > <xforms:setvalue ref="/equation/result" value="/equation/first * /equation/second"/> > <xforms:setvalue ref="/equation/screen" value="/equation/result"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > </xforms:action> > </xforms:trigger> > </xforms:case> > <xforms:case id="divide"> > <xforms:trigger> > <xforms:label>=</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/second" value="/equation/screenbuffer"/> > <xforms:setvalue ref="/equation/result" value="/equation/first div /equation/second"/> > <xforms:setvalue ref="/equation/screen" value="/equation/result"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > </xforms:action> > </xforms:trigger> > </xforms:case> > </xforms:switch><xforms:trigger> > <xforms:label>Clear</xforms:label> > <xforms:action ev:event="DOMActivate"> > <xforms:setvalue ref="/equation/first" value="0"/> > <xforms:setvalue ref="/equation/second" value="0"/> > <xforms:setvalue ref="/equation/result" value="0"/> > <xforms:setvalue ref="/equation/screen" value="0"/> > <xforms:setvalue ref="/equation/screenbuffer" value="0"/> > <xforms:toggle ev:event="DOMActivate" case="add"/> > </xforms:action> > </xforms:trigger></hbox></vbox></window>

Threre is a question. Element <xf:input incremental="true"/> (xformswidget-input binding) updates delegate value when 'click' event is occured. I'm not clear in one's mind why can value be changed when 'click' event is occured? Element <xf:input type="xsd:boolean"/> has the next check when 'click' event is occured: if (this.getAttribute("incremental") != "false") Should such check be presented because empty and "false" values are equivalent? if (this.getAttribute("incremental") == "true")

covered bugs: 1) xformswidget-trigger-minimal native code: onclick="if (!this._disabled) this.parentNode.dispatchDOMActivate();" should be: onclick="if (!this.parentNode._disabled) this.parentNode.dispatchDOMActivate();" 2) xformswidget-secret native code: this.inputField.readonly = this.delegate.isReadonly; should be: if(this.delegate.isReadonly) this.inputField.setAttribute('readonly', 'true'); else this.inputField.removeAttribute('readonly'); since 'readonly' property of xhtml:input doesn't work.

Since html:input type="checkbox" doesn't support 'readonly' attribute then we should use 'disabled'. xformswidget-input-boolean code: if (this.delegate.isReadonly) { this.inputField.setAttribute("readonly", "readonly"); } else { this.inputField.removeAttribute("readonly"); }

(In reply to comment #15) > Threre is a question. Element <xf:input incremental="true"/> (xformswidget-input > binding) updates delegate value when 'click' event is occured. I'm not clear in > one's mind why can value be changed when 'click' event is occured? Hmm, I see no reason for it either. I guess clicking should not do anything for an ordinary input. But it's not terribly important, as nothing will happen when the value is not changed. If you feel for it, create a bug for it ;-) >Element > <xf:input type="xsd:boolean"/> has the next check when 'click' event is occured: > > if (this.getAttribute("incremental") != "false") > > Should such check be presented because empty and "false" values are equivalent? > > if (this.getAttribute("incremental") == "true") The difference between the checks is that the bool-input is incremental by default, as it is more intuitive to the user.

(In reply to comment #18) Ok, I agree. But has xforms spec something about incremental behaviour of bool-input or it is xforms realization feature? > If you feel for it, create a bug for it ;-) Also please note comment #16 and comment #17. I guess I'll fix this bugs in patch for this bug if you'll not be against it.

Use 'xhtml test' and 'xul test' attachments to test the patch. Patch fixes common controls (non select and select1 controls) only.

Sorry but patch has Windows line-endings again :). After patch remark I'll fix this.

(In reply to comment #19) > (In reply to comment #18) > > Ok, I agree. But has xforms spec something about incremental behaviour of > bool-input or it is xforms realization feature? > boolean inputs having incremental="true" behavior by default isn't mentioned in the spec. It is something that we observed in other processors and like Allan said, it really does seem like what a user would expect. So we do it, too.

Before fixing select and select1 controls I guess some bugs should be fixed (in instance, bug 303353) and I'll have some questions. Now <xf:select appearance="minimal"/> looks like <xf:select appearance="compact"/>. How compact select should looks? Should it looks like on image presented in specs?

(In reply to comment #19) > (In reply to comment #18) > > Ok, I agree. But has xforms spec something about incremental behaviour of > bool-input or it is xforms realization feature? > > > If you feel for it, create a bug for it ;-) > > Also please note comment #16 and comment #17. I guess I'll fix this bugs in > patch for this bug if you'll not be against it. It's a bit bigger a hassle for you to create seperate bugs, but giant "fix it all bugs" are a pain to review, and it's difficult to figure out what a patch does. So I very much prefer seperate bugs.

(In reply to comment #25) > Before fixing select and select1 controls I guess some bugs should be fixed (in > instance, bug 303353) and I'll have some questions. > > Now <xf:select appearance="minimal"/> looks like <xf:select > appearance="compact"/>. How compact select should looks? Should it looks like on > image presented in specs? Probably :) My advise is to ignore the appearance for now, and just make one appearance work.

(In reply to comment #26) > (In reply to comment #19) > > Also please note comment #16 and comment #17. I guess I'll fix this bugs in > > patch for this bug if you'll not be against it. > > It's a bit bigger a hassle for you to create seperate bugs, but giant "fix it > all bugs" are a pain to review, and it's difficult to figure out what a patch > does. So I very much prefer seperate bugs. It is a thing that I was afraid too badly. When I did a attemt to separate interface from content then I was forced to do a big changes of code. And treafore this bugs was't fixed as you understood but they gone because of code changing. Since code was changed vastly then patch review will be difficult in any case. I see three approaches for bug fixing: 1) to do as I mention (separation interface from content) 2) to write xul controls above html controls. I belive it's ugly solution. 3) to write xul controls don't linking them with xhtml controls. I guess this approach entails complexity when controls will be modified. If you see something else please say me. Please take a look at my patch, maybe it brings to you some thoughts.

(In reply to comment #28) > (In reply to comment #26) > > (In reply to comment #19) > > > > Also please note comment #16 and comment #17. I guess I'll fix this bugs in > > > patch for this bug if you'll not be against it. > > > > It's a bit bigger a hassle for you to create seperate bugs, but giant "fix it > > all bugs" are a pain to review, and it's difficult to figure out what a patch > > does. So I very much prefer seperate bugs. > > It is a thing that I was afraid too badly. When I did a attemt to separate > interface from content then I was forced to do a big changes of code. And > treafore this bugs was't fixed as you understood but they gone because of code > changing. Since code was changed vastly then patch review will be difficult in > any case. We're getting ready for another preview, so I think it would be unwise to do such big changes to the UI before that, so doing small fixes to existing controls should go to seperate bugs so we'll get it in to the preview. > I see three approaches for bug fixing: > 1) to do as I mention (separation interface from content) > 2) to write xul controls above html controls. I belive it's ugly solution. > 3) to write xul controls don't linking them with xhtml controls. I guess this > approach entails complexity when controls will be modified. > > If you see something else please say me. Please take a look at my patch, maybe > it brings to you some thoughts. I've quickly looked through your patch, and I like approach 1. If it works for XUL and HTML, it could work for other languages too which is really good. I'm not sure that I like your getControlElement though. Couldn't the functionality you expose there be normal XBL methods? Olli, what's your take on all this?

(In reply to comment #29) > I've quickly looked through your patch, and I like approach 1. If it works for > XUL and HTML, it could work for other languages too which is really good. I'm > not sure that I like your getControlElement though. Couldn't the functionality > you expose there be normal XBL methods? > XForms controls interface ask content to provide the next interface: readonly disabled value control - dom node, f.x. for focus setting If you don't like getControlElement() method then we could replace it by properties: controlReadonly controlDisabled controlValue control At first I wrote the patch using such approach. But getControlElement() pleased to me a little more. If you like to see xbl properties instead of control object then I can rewrite patch. Do you like such properties names?

(In reply to comment #29) > > We're getting ready for another preview, so I think it would be unwise to do > such big changes to the UI before that, so doing small fixes to existing > controls should go to seperate bugs so we'll get it in to the preview. > I can post covered bugs and I can to post patches for them. But I belive it is wasted labour since architecture of controls is changed and this bugs are disappeared after patch applying. In other words covered bugs can't be exists in mentioned architecture in form in what they was covered.

(In reply to comment #30) > If you like to see xbl properties instead of control object > then I can rewrite patch. Do you like such properties names? I personally think it's a lot easier and cleaner to look at, but I'm no big JS guy. What do the rest of you say?

Comment on attachment 197676 [details] [diff] [review] patch, common controls I think I like this. The change to the current architecture isn't so big. But before accepting the change, I'd like to see some SVG widgets too and in such way that one can dynamically switch from XHTML widgets to SVG widgets. I wouldn't like to loose that feature. And of course, converting <xf:input> is much easier than for example <xf:select1>... The getControlElement is a good idea. I prefer that to the XBL methods.

(In reply to comment #32) > (In reply to comment #30) > > If you like to see xbl properties instead of control object > > then I can rewrite patch. Do you like such properties names? > > I personally think it's a lot easier and cleaner to look at, but I'm no big JS > guy. What do the rest of you say? I don't think it's more easier. Cleaner? I realy don't know. As one reason I can mention the next. Method getControlElement() looks like property 'delegate'. We have 'delegate' property to work with model and we have 'control' property to work with control. I guess it's nice solution. What do you think? Although AFAIK no one binding of mozilla tookit is realized by using js object. Maybe it is not standard approach for xbls.

(In reply to comment #33) > (From update of attachment 197676 [details] [diff] [review] [edit]) > I think I like this. The change to the current architecture isn't so big. > But before accepting the change, I'd like to see some SVG widgets too and in > such way that one can dynamically switch from XHTML widgets to SVG widgets. I > wouldn't like to loose that feature. > I don't know svg a good. Please what kind of widget you like to see. I'll try to realize it. How should looks the dynamicaly switch from xhtml widget to svg widget?

(In reply to comment #35) > > I don't know svg a good. Please what kind of widget you like to see. I'll try to > realize it. How should looks the dynamicaly switch from xhtml widget to svg widget? This is one example http://www.cs.helsinki.fi/u/pettay/moztests/xforms/dynamic_ui/calc_svg.xhtml

I loved svg :), should we include svg controls for xforms into a package as xul controls?

(In reply to comment #33) > But before accepting the change, I'd like to see some SVG widgets too and in > such way that one can dynamically switch from XHTML widgets to SVG widgets. I > wouldn't like to loose that feature. I'll write some svg controls (label, output and trigger). XForms and SVG can work together. Though I couldn't write input, secret, textarea and select controls and now I can't imagine how can I do this. Attached test 'svg test' doesn't show dynamicaly switches but I guess it will work fine.

Patch contains all XForms controls for XHTML and XUL. Select and select1 controls have common base. Also patch realizes <select1 appearance='compact'/> and <select1 appearance='full'/> controls. For testing use 'xhtml test' and 'xul test' attachments for common controls and 'xhtml test [select controls]' and 'xul test [select controls]' attachments for select and select1 controls.

Comment on attachment 199398 [details] [diff] [review] patch Finally got around to this. Sorry for the delay. It's cool work. Problem is that it does not apply cleanly on trunk (select.xml), and you need to put your new chrome files in jar.mn. Could you please supply a new patch?

Why do we need seperate .xml files for a xul and xhtml version? The select.xml for example should have all the code for xhtml, and have a xul binding in the same file overwrite what it needs. Less code duplication. Plus, not sure we need to change the way select.xml works that much for XUL to override stuff. Using a timeout is unexceptable as well.

(In reply to comment #49) > Why do we need seperate .xml files for a xul and xhtml version? > There are one reason. If base, xul and xhtml controls will be described in one file then file will be big. If some xml language (like svg) will provide own select control then select.xml file will be even greater. If you like to see xhmtl controls and xul controls in one file then I can combine into one file common controls (xforms.xml) and select controls (select.xml) and I can file a new patch. > The select.xml for example should have all the code for xhtml, and have a xul > binding in the same file overwrite what it needs. Less code duplication. Please say what code duplication do you see? > Plus, > not sure we need to change the way select.xml works that much for XUL to > override stuff. > Not only for xul. Changes allow to write select1 controls for xhtml too. Also changes allow to write small controls (please compare with select[appearance="full"]). > Using a timeout is unexceptable as well. I belive timeout is a bad solution, but i can't see a way to do it otherwise. When I try to append xul:listitem to selection then listitem binding isn't created sometimes and threafore I can't select it.

fixed style issues fixed select compact and selec1 compact in "browser mode"

(In reply to comment #51) > Created an attachment (id=199884) [edit] > patch > > fixed style issues > fixed select compact and selec1 compact in "browser mode" I've been fighting with the build bug today, so I haven't had much time too look at this. The only thing I've gotten to is that it looks like your xul:selects behaves as incremental="true" all the time... that's wrong.

fixed some problems related to selects incremental="false"

(In reply to comment #55) > > fixed some problems related to selects incremental="false" There is a problem. Element <select incremental='false' appearance='full'/> updates instance data on all internal blur events. Actually it should update instance data when focus leaves control.

Comment on attachment 200326 [details] [diff] [review] patch I'm definately not the right to review this... I suck at JS, and am not into how select and select1 is actually implemented. Olli or Doron, could you review this? My overall comments are that I like the split between core, xhtml and xul, and having native support for XUL would be a good asset. So I'm very much for this patch. There may be problems with the XUL controls, but I'm on for taking them in and fixing them as we go. We just need to make sure that XHTML works. > +++ mozilla/extensions/xforms/jar.mn 2005-10-21 17:32:36.000000000 +0900 > @@ -8,8 +8,11 @@ > * content/xforms/xforms-prefs-ui.xul (resources/content/xforms-prefs-ui.xul) > * content/xforms/xforms-prefs.js (resources/content/xforms-prefs.js) > content/xforms/xforms.xml (resources/content/xforms.xml) > - content/xforms/select1.xml (resources/content/select1.xml) > + content/xforms/xforms-xhtml.xml (resources/content/xforms-xhtml.xml) > + content/xforms/xforms-xul.xml (resources/content/xforms-xul.xml) > content/xforms/select.xml (resources/content/select.xml) > + content/xforms/select-xhtml.xml (resources/content/select-xhtml.xml) > + content/xforms/select-xul.xml (resources/content/select-xul.xml) > content/xforms/bindingex.css (resources/content/bindingex.css) > content/xforms/bindingex.xul (resources/content/bindingex.xul) > * locale/en-US/xforms/contents.rdf (resources/locale/en-US/contents.rdf) Fix the indentation > +++ mozilla/extensions/xforms/resources/content/select-xhtml.xml 2005-10-21 17:32:36.000000000 +0900 > + - Contributor(s): > + - Doron Rosenberg <doronr@us.ibm.com> > + - Alexander Surkov <surkov@dc.baikal.ru> You're not being fair to Olli here... and possibly in other files too. > + <binding id="xformswidget-select-minimal" > + extends="chrome://xforms/content/select-xhtml.xml#xformswidget-select"/> > + > + <binding id="xformswidget-select-compact" > + extends="chrome://xforms/content/select-xhtml.xml#xformswidget-select"/> > + > + <binding id="xformswidget-select-full" > + extends="chrome://xforms/content/select-xhtml.xml#xformswidget-select"/> Why are these necessary? Can't you just bind all select controls to xformswidget-select? > + <!-- WIDGET FOR SELECT APPEARANCE='COMPACT' --> > + <binding id="widget-select-compact" > + extends="chrome://xforms/content/select.xml#widget-base"> > + > + <content> > + <html:select xbl:inherits="style, disabled=readonly" > + multiple="true" > + size="5" > + anonid="control"/> > + </content> > + > + <implementation> > + <property name="readonly" > + onget="return this.getAttribute('readonly' == 'true') ? true : false;" > + onset="if(val) this.setAttribute('readonly', 'true'); else this.removeAttribute('readonly');"/> What's this? There should be no readonly attribute on xforms:select. > +++ mozilla/extensions/xforms/resources/content/xforms.css 2005-10-21 17:32:35.000000000 +0900 > @@ -45,10 +46,7 @@ > contextcontainer, > group, > switch, > -case, > -select1 item, > -select1 itemset, > -select1 choices { > +case{ case { > -input { > - -moz-binding: url('chrome://xforms/content/xforms.xml#xformswidget-input'); > +xul|*:root output { > + -moz-binding: url('chrome://xforms/content/xforms-xul.xml#xformswidget-output'); > + display: -moz-box; > } What does "display: -moz-box" do? > -/* Most of the select1 specific CSS is copied from forms.css */ > - > -select1 item { > - -moz-binding: url('chrome://xforms/content/select1.xml#xformswidget-select1-item'); > - white-space : nowrap; > +xul|*:root trigger, xul|*:root submit { > + -moz-binding: url('chrome://xforms/content/xforms-xul.xml#xformswidget-trigger'); > } > -html|input.-moz-xforms-select1-dropdown { > - width: 12px; > - height: 1.3em; > - white-space: nowrap; > - position: static !important; > - background-image: url("data:image/gif;base64,R0lGODlhBwAEAIAAAAAAAP%2F%2F%2FyH5BAEAAAEALAAAAAAHAAQAAAIIhA%2BBGWoNWSgAOw%3D%3D") !important; > - background-repeat: no-repeat !important; > - background-position: center !important; > - -moz-appearance: menulist-button; > - -moz-user-select: none !important; > - -moz-user-focus: ignore !important; > - -moz-binding: none; > - vertical-align: text-top; > - margin: 0px !important; > - margin-top: -1px !important; Why isn't all this magic needed anymore?

(In reply to comment #57) > Fix the indentation > What exactly should I do? > Why are these necessary? Can't you just bind all select controls to > xformswidget-select? When appearance attribute is changed then we should call nsIXFormsDelegate.widgetAttached() method. When we have separate binding for each appearance attibute value then it was happen automatically. I tried to realize this by using DOMAttrModified event but I was failed because binding for internal control is not changed yet. > What's this? There should be no readonly attribute on xforms:select. > There is no 'readonly' attribute on xforms:select. There is 'readonly' attribute on internal element of xfroms:select. > What does "display: -moz-box" do? > Custom namespace element looks like xul:box element. There are some layout problems for custom ns elements in xul context. > > Why isn't all this magic needed anymore? > Since select1.xml isn't used then it's styles are useless.

Comment on attachment 200599 [details] [diff] [review] patch I didn't test this, but if I read the patch correctly, it breaks <select1 selection="open"/>. Right? selection="open" was the main reason to implement <select1> as a "dhtml widget".

(In reply to comment #61) > (From update of attachment 200599 [details] [diff] [review] [edit]) > I didn't test this, but if I read the patch correctly, it breaks <select1 > selection="open"/>. > Right? > > selection="open" was the main reason to implement <select1> as a "dhtml > widget". > Yes, you're right, I forgot about it. As I understand you want selection='open' realization for <select1 appearance='minimal'/>. Am I right? There is a quick way to support this: xhtml minimal select1 leaving as is. Maybe it isn't so nice solution but it isn't bad. Because now there is no open selection realization for select, select1 controls (except minimal select1) and at first we should define how should them behave. How do you think?

Patch doesn't remove select1 appearance='minimal' for xhtml context. Now selection='open' works for it. I think it's better to file new bug about supporting open selection for other controls because I belive small patches is better than big.

hmm .. any idea what can we do with this ? this patch looks good but as said we'll need to fill another bug and come up with a small patch later. I don't think it will make 1.5 except if they decide to check this in before after RC1 release for RC2. But definately, it's something i would like to have before 1.5 . What you guys plan for this one ?

(In reply to comment #64) > hmm .. any idea what can we do with this ? this patch looks good but as said > we'll need to fill another bug and come up with a small patch later. I don't > think it will make 1.5 except if they decide to check this in before after RC1 > release for RC2. But definately, it's something i would like to have before 1.5 > . What you guys plan for this one ? > OMG, i'm still a bit asleep i thought it was a branch bug. Really sorry for these two unneeded auto-CC's :S

Comment on attachment 200599 [details] [diff] [review] patch I'll review the patch with select1

Comment on attachment 200714 [details] [diff] [review] patch (with select1) please use spaces, not tabs.

Comment on attachment 200714 [details] [diff] [review] patch (with select1) (In reply to comment #67) > (From update of attachment 200714 [details] [diff] [review] [edit]) > please use spaces, not tabs. > And could you fix indentations and other syntax style problems. It is somewhat difficult to read the current patch. http://www.mozilla.org/hacking/mozilla-style-guide.html is mainly for C++, but should give a hint ;)

fixed code style issues

updated to current build

Comment on attachment 201494 [details] [diff] [review] patch This may sound nitpicking to you, but still comments about coding style;) Right style makes code so much more readable. The syntax for if-else should be if (expression) statement; else statement; or if (expression) { 1_or_more_statements; } else { 1_or_more_statements; } Indentation is 2 spaces. If attributes can't be put to the same line, they should be handled like this (if possible): <element attr="val" attr2="val2"/> I'll try to really review the code tomorrow, but marking r- because of styling issues (at least because of those).

fixed style issues

Comment on attachment 201603 [details] [diff] [review] patch The patch doesn't use the new version of Custom Control API (Bug 306764) And btw, I found still at least one tab in the patch. >+ >+ <binding id="xformswidget-select-minimal" >+ extends="chrome://xforms/content/select-xhtml.xml#xformswidget-select"/> >+ >+ <binding id="xformswidget-select-compact" >+ extends="chrome://xforms/content/select-xhtml.xml#xformswidget-select"/> >+ >+ <binding id="xformswidget-select-full" >+ extends="chrome://xforms/content/select-xhtml.xml#xformswidget-select"/> >+ The reason for these bindings is not quite clear to me. >+ >+ <handlers> >+ <handler event="focus" phase="capturing"> >+ this.dispatchDOMUIEvent("DOMFocusIn"); >+ </handler> >+ >+ <handler event="blur" phase="capturing"> >+ this.updateModelData(); >+ this.dispatchDOMUIEvent("DOMFocusOut"); >+ </handler> >+ >+ <handler event="change"> >+ this.updateModelDataIncremental(); >+ </handler> >+ </handlers> If possible, I'd attach event listeners to the element which is the originalTarget of the event. So usually it is some element in the anonymous content. >+ <!-- WIDGET FOR SELECT1 APPEARANCE='MINIMAL' --> >+ <binding id="widget-select1-minimal" >+ extends="chrome://xforms/content/select-xhtml.xml#widget-select-compact"> >+ >+ <content> >+ <html:select xbl:inherits="style, disabled=readonly" anonid="control"/> >+ </content> Are you still using html:select for some version of <xforms:select1> ? Any reason for that. I think in XHTML we should use the one we already have and XUL should have its own widget which supports the features <xforms:select1 selection="open" appearance="minimal"/> >+ >+ <!-- WIDGET SELECT1 APPEARANCE='FULL' --> >+ <binding id="widget-select1-full" >+ extends="chrome://xforms/content/select-xhtml.xml#widget-select-full"> >+ >+ <content> >+ <html:form anonid="control"/> >+ </content> Why do you need <html:form> here? More comments later. Still haven't tested this, because it doesn't work with the current trunk.

(In reply to comment #73) > (From update of attachment 201603 [details] [diff] [review] [edit]) > The patch doesn't use the new version of Custom Control API (Bug 306764) I updated patch to current build but I can't to check it because I can't find latest xforms package for windows (I checked 5-6 November, ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/). > And btw, I found still at least one tab in the patch. > I found one tab in the patch. Fixed. > >+ > >+ <binding id="xformswidget-select-minimal" > >+ extends="chrome://xforms/content/select-xhtml.xml#xformswidget-select"/> > >+ > >+ <binding id="xformswidget-select-compact" > >+ extends="chrome://xforms/content/select-xhtml.xml#xformswidget-select"/> > >+ > >+ <binding id="xformswidget-select-full" > >+ extends="chrome://xforms/content/select-xhtml.xml#xformswidget-select"/> > >+ > > The reason for these bindings is not quite clear to me. > I added comment about it into the patch. The reason why I do it is the following. When appearance attribute is changed then we should call nsIXFormsDelegate.widgetAttached() method. If we will have separate binding for each appearance attibute value then it will be happen automatically. I tried to realize this by using DOMAttrModified event but I was failed because binding for internal control is not changed when DOMAttrModified is handled. > > >+ > >+ <handlers> > >+ <handler event="focus" phase="capturing"> > >+ this.dispatchDOMUIEvent("DOMFocusIn"); > >+ </handler> > >+ > >+ <handler event="blur" phase="capturing"> > >+ this.updateModelData(); > >+ this.dispatchDOMUIEvent("DOMFocusOut"); > >+ </handler> > >+ > >+ <handler event="change"> > >+ this.updateModelDataIncremental(); > >+ </handler> > >+ </handlers> > > > If possible, I'd attach event listeners to the element which is the > originalTarget > of the event. So usually it is some element in the anonymous content. > It is possible for full select and select1 full. I added the check for it. It's some tricly for other selects and I didn't do it. > >+ <!-- WIDGET FOR SELECT1 APPEARANCE='MINIMAL --> > Are you still using html:select for some version of <xforms:select1> ? > Any reason for that. I think in XHTML we should use the one > we already have and XUL should have its own widget which supports the > features <xforms:select1 selection="open" appearance="minimal"/> > > No, I don't use. I forgot to remove this code. > >+ > >+ <!-- WIDGET SELECT1 APPEARANCE='FULL' --> > >+ <binding id="widget-select1-full" > >+ extends="chrome://xforms/content/select-xhtml.xml#widget-select-full"> > >+ > >+ <content> > >+ <html:form anonid="control"/> > >+ </content> > > Why do you need <html:form> here? Radio buttons with unique name works in coordination when they are placed in one form. > > More comments later. Still haven't tested this, because it doesn't work with > the current > trunk. > I'll update the patch but as I said I can't be sure on it. When I'll get latest patch then I'll check it.

I updated patch to current build but I couldn't check it because I didn't find latest xforms package for windows (I checked 5-6 November, ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/).

Comment on attachment 202068 [details] [diff] [review] patch I get the following error when using <xf:select> (which has both <xf:item>s and <xf:choices>s) inside <xf:repeat> ( testcase http://www.cs.helsinki.fi/u/pettay/moztests/xforms/nested_select.xhtml ) * Call to xpconnect wrapped JSObject produced this error: * [Exception... "'[JavaScript Error: "this.buildChoices is not a function" {file: "chrome://xforms/content/select.xml" line: 326}]' when calling method: [nsIXForm sUIWidget::refresh]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_ DETAILS)" location: "<unknown>" data: yes] I get also: JavaScript error: chrome://xforms/content/xforms.xml, line 230: syntax error Check spelling in the comments. I didn't test the XUL widgets at all yet. Perhaps you have testcases, which you could put to some website? >+<!-- SELECT, SELECT1 CONTROLS --> >+ <binding id="xformswidget-select" >+ extends="chrome://xforms/content/select.xml#xformswidget-select-base"> >+ >+ <content> >+ <html:table xbl:inherits="style"> >+ <html:tr> >+ <html:td valign="top"><children includes="label"/></html:td> >+ <html:td><html:selectwidget anonid="control" class="xf-value"/></html:td> >+ </html:tr> >+ </html:table> >+ <children/> >+ </content> Is the html:table used now for all <select>s? Earlier it was just for <xforms:select appearance="full"/>, and even that was a bad choice, IMO. >+ <method name="getSelectedValues"> >+ <body><![CDATA[ >+ var values=[]; >+ >+ var items = >+ this.control.getElementsByTagNameNS(this.XHTML_NS, "input"); >+ for(var i = 0; i < items.length; i++) { >+ if (items[i].checked) >+ values.push(items[i].value); >+ } >+ return values; >+ ]]></body> >+ </method> newline after <body> and before </body> and use the same style for CDATA sections also elsewhere. and more comments later ;)

(In reply to comment #76) > (From update of attachment 202068 [details] [diff] [review] [edit]) > I get the following error when using <xf:select> (which has both <xf:item>s and > <xf:choices>s) inside <xf:repeat> > ( testcase > http://www.cs.helsinki.fi/u/pettay/moztests/xforms/nested_select.xhtml ) > * Call to xpconnect wrapped JSObject produced this error: * > [Exception... "'[JavaScript Error: "this.buildChoices is not a function" {file: > "chrome://xforms/content/select.xml" line: 326}]' when calling method: > [nsIXForm sUIWidget::refresh]" nsresult: "0x80570021 > (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_ DETAILS)" location: "<unknown>" data: > yes] > > I get also: > JavaScript error: chrome://xforms/content/xforms.xml, line 230: syntax error > Fixed > Check spelling in the comments. > Sorry, I forgot to fix this. I'll fix it in the next patch. > I didn't test the XUL widgets at all yet. Perhaps you have > testcases, which you could put to some website? > Simple xul tests are attached to the bug: "xul test" and "xul test [select controls]" attacments. If you need something more, please tell me, I can expand tests. > >+<!-- SELECT, SELECT1 CONTROLS --> > >+ <binding id="xformswidget-select" > >+ extends="chrome://xforms/content/select.xml#xformswidget-select-base"> > >+ > >+ <content> > >+ <html:table xbl:inherits="style"> > >+ <html:tr> > >+ <html:td valign="top"><children includes="label"/></html:td> > >+ <html:td><html:selectwidget anonid="control" class="xf-value"/></html:td> > >+ </html:tr> > >+ </html:table> > >+ <children/> > >+ </content> > > Is the html:table used now for all <select>s? Earlier it was just for > <xforms:select appearance="full"/>, and even that was a bad choice, IMO. > Yes, for all. If I understand right then we should have the next layout for all select controls: |-------|---------| | label | selitem | | | selitem | | | selitem | |-------|---------| Table does it a simple and quick. If you prefer another way then please let me know. I havn't any special preferences. > > >+ <method name="getSelectedValues"> > >+ <body><![CDATA[ > >+ var values=[]; > >+ > >+ var items = > >+ this.control.getElementsByTagNameNS(this.XHTML_NS, "input"); > >+ for(var i = 0; i < items.length; i++) { > >+ if (items[i].checked) > >+ values.push(items[i].value); > >+ } > >+ return values; > >+ ]]></body> > >+ </method> > > newline after <body> and before </body> > and use the same style for CDATA sections also elsewhere. > Fixed > and more comments later ;) >

(In reply to comment #78) > (In reply to comment #76) > > (From update of attachment 202068 [details] [diff] [review] [edit] [edit]) > > Is the html:table used now for all <select>s? Earlier it was just for > > <xforms:select appearance="full"/>, and even that was a bad choice, IMO. > > > > Yes, for all. If I understand right then we should have the next layout for all > select controls: > > |-------|---------| > | label | selitem | > | | selitem | > | | selitem | > |-------|---------| > > Table does it a simple and quick. If you prefer another way then please let me > know. I havn't any special preferences. > With tables we can't handle for example the following layout: http://www.cs.helsinki.fi/u/pettay/moztests/xforms/select_inline.xhtml

(In reply to comment #79) > With tables we can't handle for example the following layout: > http://www.cs.helsinki.fi/u/pettay/moztests/xforms/select_inline.xhtml > I think it can be fixed by setting "display: inline" style. The above example shows <label/> element at bottom of <select/> control. W3c draws <label/> at the top of <select/> (http://www.w3.org/TR/2005/PER-xforms-20051006/slice8.html#ui-selectMany). Where should <label/> be placed?

(In reply to comment #80) > (In reply to comment #79) > > > With tables we can't handle for example the following layout: > > http://www.cs.helsinki.fi/u/pettay/moztests/xforms/select_inline.xhtml > > > > I think it can be fixed by setting "display: inline" style. The above example > shows <label/> element at bottom of <select/> control. W3c draws <label/> at > the top of <select/> > (http://www.w3.org/TR/2005/PER-xforms-20051006/slice8.html#ui-selectMany). > Where should <label/> be placed? > No, I cant achive desired behaviour with "display: inline" style. As before I think <label/> should be placed at the top of <select/> control. If you agree with it then maybe you know how can it be achived without <table/> using.

(In reply to comment #81) > > No, I cant achive desired behaviour with "display: inline" style. As before I > think <label/> should be placed at the top of <select/> control. If you agree > with it then maybe you know how can it be achived without <table/> using. > There are two cases in the example, the <label> at the top and bottom. With the current <xf:select> implementation we can support them both.

Now select controls use layouts as before.

(In reply to comment #83) > Now select controls use layouts as before. > No it does not. Re-test with http://www.cs.helsinki.fi/u/pettay/moztests/xforms/select_inline.xhtml I added two test cases and more comments to the original tests. If you can produce same kind of layout with the patch, could you tell how ;)

Test http://www.cs.helsinki.fi/u/pettay/moztests/xforms/select_inline.xhtml works as it should, I hope :).

updated to current build

updated

(In reply to comment #88) > Created an attachment (id=204753) [edit] > patch > > updated > Can you verify that this is updated to the latest level of code? This patch seems to remove all of my copy handling code in select.xml

(In reply to comment #89) > > Can you verify that this is updated to the latest level of code? This patch > seems to remove all of my copy handling code in select.xml > You're right. I'll fix it and verify patch update soon.

updated

I'll split the bug into several small bugs. I belive the patches for them will be easer to review.

Comment on attachment 206669 [details] [diff] [review] patch So I guess I can clear this review flag.

(In reply to comment #93) > I'll split the bug into several small bugs. I belive the patches for them will > be easer to review. Mighty good idea :) And keep up the good work.

We should probably file this as a dupe of the new bugs, or?

(In reply to comment #96) > We should probably file this as a dupe of the new bugs, or? > Probably we should set fixed when new bugs will be fixed.

(In reply to comment #97) > (In reply to comment #96) > > We should probably file this as a dupe of the new bugs, or? > > > > Probably we should set fixed when new bugs will be fixed. I've "[Meta]"'ed it.

Mark as fixed since main xul controls were implemented and bugs blocking this one were fixed. Please use the bug 327236 to track the work under rest of xul controls implementing.