Closed Bug 284068 Opened 19 years ago Closed 18 years ago

clicking on xf:label should activate xf:input/xhtml:input

Categories

(Core Graveyard :: XForms, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: annevk, Assigned: surkov)

References

Details

(Keywords: fixed1.8.0.8, fixed1.8.1.1, testcase)

Attachments

(4 files, 4 obsolete files)

Although the XForms specification does not mention it, I think XForms should act
similar to HTML 4.01 in this aspect. It is very useful for usability and
accessibility of form controls.
Summary: clicking on xf:label should active xf:input/xhtml:input → clicking on xf:label should activate xf:input/xhtml:input
Attached file testcase
Keywords: testcase
It should also toggle checkboxes, etc.
Assignee: aaronr → allan
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
Here's a go at it.

This doesn't fix attachment 175815 [details] though, as the input is not bound to a node
=>  is not relevant => is not focusable. I haven't actually checked the spec.,
but it's a different problem.

(I've also added an error message to setfocus, if it points to something wrong)
Attachment #185148 - Flags: review?(smaug)
(In reply to comment #2)
> It should also toggle checkboxes, etc.

Should it now? :-)

Yes, it probably should. But "etc." == ?

/me should read the entire bug before hacking...
Attachment #185148 - Flags: review?(smaug)
Attached patch another possible approach (obsolete) — Splinter Review
Attachment #201012 - Flags: review?(allan)
Comment on attachment 201012 [details] [diff] [review]
another possible approach

As Leigh writes in comment 2 it should also check/uncheck checkboxes. That is for bool inputs.
Attachment #201012 - Flags: review?(allan) → review-
Attached patch kick checkboxes too (obsolete) — Splinter Review
Attachment #201206 - Flags: review?(allan)
Attachment #201206 - Flags: review?(allan) → review+
Attachment #201206 - Flags: review?(doronr)
Attachment #201206 - Flags: review?(doronr) → review+
this patch does work with the boolean input testcase when you click on the label of the checkbox.  But if you click on the checkbox itself, it doesn't check/uncheck correctly.
(In reply to comment #10)
> this patch does work with the boolean input testcase when you click on the
> label of the checkbox.  But if you click on the checkbox itself, it doesn't
> check/uncheck correctly.

heh! haven't checked myself... but that _could_ be a useful feature ;-)
Blocks: 326372
Blocks: 326373
It's a wrong way to handle click event on xforms:input control because xforms:input can contain not only labels controls.
Attachment #201206 - Flags: review+ → review-
Assignee: allan → aaronr
Status: ASSIGNED → NEW
Severity: normal → enhancement
Mark as blocking of bug 337250 since I guess it's accessibility issue.
Blocks: 337250
Sorry, I have in view the bug 337249.
Blocks: xformsa11y
No longer blocks: 337250
Assignee: aaronr → xforms
Probably we should support that stuff not only for xf:inputs? F.x xf:select/xf:select1? Probably for all Form Controls?
Assignee: xforms → surkov.alexander
(In reply to comment #15)
> Probably we should support that stuff not only for xf:inputs? F.x
> xf:select/xf:select1? Probably for all Form Controls?

Good idea.
Attached patch patch (obsolete) — Splinter Review
implementation for xul and xhtml have dublicate code: click handler. I don't like it a much but I don't like more to add event handlers in interface bindings. I added base label binding for xul because I want to see a accesskey label widget for xul too (the bug 340088).
Attachment #185148 - Attachment is obsolete: true
Attachment #201206 - Attachment is obsolete: true
Attachment #224202 - Flags: review?(allan)
Status: NEW → ASSIGNED
Attachment #201012 - Attachment is obsolete: true
Comment on attachment 224202 [details] [diff] [review]
patch

Aaron, probably you'll do review until Allan is ready for reviews.
Attachment #224202 - Flags: review?(aaronr)
(In reply to comment #18)
> (From update of attachment 224202 [details] [diff] [review] [edit])
> Aaron, probably you'll do review until Allan is ready for reviews.
> 

The code looks good.  My only questions are about the organization.  I don't see why you have the code duplication.  Any label with a visual representation will need a click handler.  If nothing else, just move it to xforms.xml instead of having it both in xforms-xhtml.xml and xforms-xul.xml.

Also, maybe call it advanceFocusToParent() instead of adviceFocusToParent().  You should also comment this function or open another bug so that we remember to use mozType:baseType instead of mozType:type once that is available.  Though I can't see why someone would extend boolean :=)
Attached patch patch2Splinter Review
(In reply to comment #19)
> (In reply to comment #18)
> > (From update of attachment 224202 [details] [diff] [review] [edit] [edit])
> > Aaron, probably you'll do review until Allan is ready for reviews.
> > 
> 
> The code looks good.  My only questions are about the organization.  I don't
> see why you have the code duplication.  Any label with a visual representation
> will need a click handler.  If nothing else, just move it to xforms.xml instead
> of having it both in xforms-xhtml.xml and xforms-xul.xml.

Agree.

> Also, maybe call it advanceFocusToParent() instead of adviceFocusToParent(). 

It's my misspeling :). For sure "advance", at least, it's anology with xul command dispatcher.

> You should also comment this function or open another bug so that we remember
> to use mozType:baseType instead of mozType:type once that is available.  Though
> I can't see why someone would extend boolean :=)
> 

Yes, I added XXX section linking on bug 316691.
Attachment #224202 - Attachment is obsolete: true
Attachment #228924 - Flags: review?(aaronr)
Attachment #224202 - Flags: review?(allan)
Attachment #224202 - Flags: review?(aaronr)
Comment on attachment 228924 [details] [diff] [review]
patch2

could you open a bug on the fact that select doesn't focus right?  All the other controls that I tried with this patch worked fine.
Attachment #228924 - Flags: review?(aaronr) → review+
(In reply to comment #21)
> (From update of attachment 228924 [details] [diff] [review] [edit])
> could you open a bug on the fact that select doesn't focus right?  All the
> other controls that I tried with this patch worked fine.
> 

I filed bug 344379.
Attachment #228924 - Flags: review?(doronr)
Attachment #228924 - Flags: review?(doronr) → review+
checked into trunk for surkov
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
checked into 1.8.0 branch on 2006/09/21
Keywords: fixed1.8.0.8
checked into 1.8 branch on 2006/11/21
Keywords: fixed1.8.1.1
Whiteboard: xf-to-branch
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: