Closed
Bug 323850
Opened 19 years ago
Closed 19 years ago
XForms widgets (xforms.xml) for xul
Categories
(Core Graveyard :: XForms, defect)
Core Graveyard
XForms
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: surkov, Assigned: surkov)
References
Details
(Keywords: fixed1.8.0.4, fixed1.8.1)
Attachments
(2 files, 3 obsolete files)
|
(deleted),
application/vnd.mozilla.xul+xml
|
Details | |
|
(deleted),
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051212 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051212 Firefox/1.6a1
The patch coming up.
Reproducible: Always
|
||
Updated•19 years ago
|
Assignee: aaronr → surkov
|
|
||
Updated•19 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
|
Assignee | |
Comment 1•19 years ago
|
||
*** Bug 323006 has been marked as a duplicate of this bug. ***
|
|
Assignee | |
Comment 2•19 years ago
|
||
|
|
Assignee | |
Comment 3•19 years ago
|
||
Attachment #211942 -
Flags: review?(smaug)
|
|
Assignee | |
Updated•19 years ago
|
Attachment #211942 -
Flags: review?(allan)
|
|
||
Comment 4•19 years ago
|
||
Comment on attachment 211942 [details] [diff] [review]
patch
Code-wise I mostly have nits, except for the use of the html:input.
> +++ mozilla/extensions/xforms/resources/content/xforms.css 2006-02-15 12:22:00.000000000 +0800
> +xul|*:root output {
> + -moz-binding: url('chrome://xforms/content/xforms-xul.xml#xformswidget-output');
> + display: -moz-box;
> +}
I'm a bit uncertain on whether all styling should go here, or directly on the control. It's a design issue I guess. We talked about it on on IRC, and for theming issues we might want to stash everything in CSS... Different opinions anyone?
> +xul|*:root secret html|input[readonly] {
> + background-color: -moz-Dialog;
> + color: -moz-DialogText;
> +}
No XUL "secret" control?
> +++ mozilla/extensions/xforms/resources/content/xforms-xul.xml 2006-02-15 12:22:00.000000000 +0800
> + - The Original Code is Mozilla XForms support.
> + -
> + - The Initial Developer of the Original Code is
> + - Novell, Inc.
Well, congrats on your job at Novell ;)
> + - Portions created by the Initial Developer are Copyright (C) 2005
> + - the Initial Developer. All Rights Reserved.
I konw you are in a different time zone, but in a different year?! ;)
> +<bindings id="xformsBindings"
> + xmlns="http://www.mozilla.org/xbl"
> + xmlns:html="http://www.w3.org/1999/xhtml"
remember that the xhtml namespace should not be needed, when you kill the input type="secret"
> + <!-- LABEL: <DEFAULT> -->
> + <binding id="xformswidget-label"
> + extends="chrome://xforms/content/xforms.xml#xformswidget-label-base">
> + <content>
> + <xul:deck anonid="activecontent" flex="1">
I do not like the name "activecontent", it sort of implied that it is either that or "implicitcontent" that is shown or something... How about just "deck" for example, or "contentswitcher"?
> + <binding id="xformswidget-input-base"
> + extends="chrome://xforms/content/xforms.xml#xformswidget-input-base">
> + <handler event="input">
> + <![CDATA[
why the CDATA?
> + <binding id="xformswidget-input"
> + extends="chrome://xforms/content/xforms-xul.xml#xformswidget-input-base">
I do not think you need an absolute path. Isn't "#xformswidget-input-base" enough? This might be elsewhere too.
> + <implementation>
> + <method name="getControlElement">
> + <body>
> + var control = this.ownerDocument.
> + getAnonymousElementByAttribute(this, 'anonid', 'control');
> +
> + return {
> + control: control,
please add an empty line here. Makes it easier to read
> + <handlers>
> + <handler event="keypress" keycode="VK_RETURN">
> + <![CDATA[
again, CDATA, why?
> + <!-- SECRET: <DEFAULT> -->
> + <binding id="xformswidget-secret"
> + extends="chrome://xforms/content/xforms-xul.xml#xformswidget-input-base">
> + <method name="getControlElement">
> + <body>
> + return {
> + __proto__: this.ownerDocument.
> + getAnonymousElementByAttribute(this, 'anonid', 'control'),
> +
> + set readonly(val) {
> + if (val) {
> + this.setAttribute('readonly', 'true');
Should be "readonly" instead of "true", but this will probably change when you use XUL control instead.
> + <binding id="xformswidget-textarea"
> + extends="chrome://xforms/content/xforms-xul.xml#xformswidget-input-base">
> + <method name="getControlElement">
> + <body>
> + var control = this.ownerDocument.
> + getAnonymousElementByAttribute(this, 'anonid', 'control');
> +
> + return {
> + control: control,
newline
> + <binding id="xformswidget-trigger" display="xul:button"
> + extends="chrome://xforms/content/xforms-xul.xml#xformswidget-trigger-base">
> + <handle event="command">
> + // XXX: we should fire 'DOMActivate' event for properly xforms:submit
> + // work since xul:button don't do it (see a bug 323005
// XXX: we need to fire 'DOMActivate' event to get xforms:submit
// to work, since xul:button do not do it (see a bug 323005
Functionality-wise:
* I think the minimal trigger should have some styling that indicates that you can click on it
* Nothing seems to happen when I click a trigger. As is: I get no visual indication of my click. And the trigger does not get focus, which it should.
Attachment #211942 -
Flags: review?(smaug)
Attachment #211942 -
Flags: review?(allan)
Attachment #211942 -
Flags: review-
|
|
Assignee | |
Comment 5•19 years ago
|
||
with allan's comments
Attachment #211942 -
Attachment is obsolete: true
Attachment #212107 -
Flags: review?(allan)
|
|
Assignee | |
Updated•19 years ago
|
Attachment #212107 -
Flags: review?(smaug)
|
|
||
Comment 6•19 years ago
|
||
Comment on attachment 212107 [details] [diff] [review]
patch
Nice. I still want a visual indication of that you can click on a minimal trigger, without hovering over it. Can you not underline it or something?
with that, r=me
Attachment #212107 -
Flags: review?(allan) → review+
|
|
||
Comment 7•19 years ago
|
||
(In reply to comment #6)
> (From update of attachment 212107 [details] [diff] [review] [edit])
> Nice. I still want a visual indication of that you can click on a minimal
> trigger, without hovering over it. Can you not underline it or something?
>
> with that, r=me
>
Ok, XUL toolbar buttons do not have that normally, so forget that. r=me without any additional requirements.
|
|
Assignee | |
Comment 8•19 years ago
|
||
Attachment #212107 -
Attachment is obsolete: true
Attachment #212462 -
Flags: review?(smaug)
Attachment #212107 -
Flags: review?(smaug)
|
||
Comment 9•19 years ago
|
||
Comment on attachment 212462 [details] [diff] [review]
up-to-date
Hmm, is there something wrong in my build or why the input elements don't work... :(
|
|
Assignee | |
Comment 10•19 years ago
|
||
(In reply to comment #9)
> (From update of attachment 212462 [details] [diff] [review] [edit])
> Hmm, is there something wrong in my build or why the input elements don't
> work... :(
>
I'm sorry. I did wrong update. Now it's fixed.
Attachment #212462 -
Attachment is obsolete: true
Attachment #212547 -
Flags: review?(smaug)
Attachment #212462 -
Flags: review?(smaug)
|
|
||
Updated•19 years ago
|
Attachment #212547 -
Flags: review?(smaug) → review+
|
|
||
Comment 11•19 years ago
|
||
Checked in
Checking in jar.mn;
/cvsroot/mozilla/extensions/xforms/jar.mn,v <-- jar.mn
new revision: 1.13; previous revision: 1.12
done
RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/input-xul.xml,v
done
Checking in resources/content/input-xul.xml;
/cvsroot/mozilla/extensions/xforms/resources/content/input-xul.xml,v <-- input-xul.xml
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/extensions/xforms/resources/content/xforms-xul.xml,v
done
Checking in resources/content/xforms-xul.xml;
/cvsroot/mozilla/extensions/xforms/resources/content/xforms-xul.xml,v <-- xforms-xul.xml
initial revision: 1.1
done
Checking in resources/content/xforms.css;
/cvsroot/mozilla/extensions/xforms/resources/content/xforms.css,v <-- xforms.css
new revision: 1.15; previous revision: 1.14
done
Whiteboard: xf-to-branch
|
|
||
Updated•19 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
|
|
||
Updated•19 years ago
|
Keywords: fixed1.8.0.3,
fixed1.8.1
|
|
||
Updated•19 years ago
|
Whiteboard: xf-to-branch
|
||
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•