Last Comment Bug 284068 - clicking on xf:label should activate xf:input/xhtml:input
: clicking on xf:label should activate xf:input/xhtml:input
Status: RESOLVED FIXED
: fixed1.8.0.8, fixed1.8.1.1, testcase
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: ---
Assigned To: alexander :surkov
: Stephen Pride
:
Mentors:
Depends on:
Blocks: 326372 326373 xformsa11y
  Show dependency treegraph
 
Reported: 2005-02-28 04:37 PST by Anne (:annevk)
Modified: 2016-07-15 14:46 PDT (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
testcase (484 bytes, application/xml)
2005-02-28 04:41 PST, Anne (:annevk)
no flags Details
Testcase using setfocus too (2.37 KB, application/xhtml+xml)
2005-06-02 06:54 PDT, Allan Beaufour
no flags Details
Patch (3.90 KB, patch)
2005-06-02 06:56 PDT, Allan Beaufour
no flags Details | Diff | Splinter Review
another possible approach (1.95 KB, patch)
2005-10-27 10:43 PDT, tor
allan: review-
Details | Diff | Splinter Review
kick checkboxes too (2.62 KB, patch)
2005-10-28 16:12 PDT, tor
allan: review-
doronr: review+
Details | Diff | Splinter Review
Testcase for boolean input (935 bytes, application/xhtml+xml)
2005-11-01 04:20 PST, Allan Beaufour
no flags Details
patch (5.46 KB, patch)
2006-06-02 08:40 PDT, alexander :surkov
no flags Details | Diff | Splinter Review
patch2 (2.15 KB, patch)
2006-07-12 00:54 PDT, alexander :surkov
aaronr: review+
doronr: review+
Details | Diff | Splinter Review

Description Anne (:annevk) 2005-02-28 04:37:34 PST
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.
Comment 1 Anne (:annevk) 2005-02-28 04:41:16 PST
Created attachment 175815 [details]
testcase
Comment 2 Leigh L. Klotz, Jr. 2005-03-08 14:31:49 PST
It should also toggle checkboxes, etc.
Comment 3 Allan Beaufour 2005-06-02 06:54:09 PDT
Created attachment 185147 [details]
Testcase using setfocus too
Comment 4 Allan Beaufour 2005-06-02 06:56:24 PDT
Created attachment 185148 [details] [diff] [review]
Patch

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)
Comment 5 Allan Beaufour 2005-06-02 06:57:36 PDT
(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...
Comment 6 tor 2005-10-27 10:43:13 PDT
Created attachment 201012 [details] [diff] [review]
another possible approach
Comment 7 Allan Beaufour 2005-10-28 14:32:13 PDT
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.
Comment 8 tor 2005-10-28 16:12:20 PDT
Created attachment 201206 [details] [diff] [review]
kick checkboxes too
Comment 9 Allan Beaufour 2005-11-01 04:20:13 PST
Created attachment 201501 [details]
Testcase for boolean input
Comment 10 aaronr 2005-11-02 15:35:59 PST
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.
Comment 11 Allan Beaufour 2005-11-03 00:09:34 PST
(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 ;-)
Comment 12 alexander :surkov 2006-03-13 23:19:14 PST
It's a wrong way to handle click event on xforms:input control because xforms:input can contain not only labels controls.
Comment 13 alexander :surkov 2006-05-09 00:53:29 PDT
Mark as blocking of bug 337250 since I guess it's accessibility issue.
Comment 14 alexander :surkov 2006-05-09 00:55:42 PDT
Sorry, I have in view the bug 337249.
Comment 15 alexander :surkov 2006-06-02 03:53:33 PDT
Probably we should support that stuff not only for xf:inputs? F.x xf:select/xf:select1? Probably for all Form Controls?
Comment 16 Allan Beaufour 2006-06-02 03:56:48 PDT
(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.
Comment 17 alexander :surkov 2006-06-02 08:40:54 PDT
Created attachment 224202 [details] [diff] [review]
patch

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).
Comment 18 alexander :surkov 2006-07-08 20:47:05 PDT
Comment on attachment 224202 [details] [diff] [review]
patch

Aaron, probably you'll do review until Allan is ready for reviews.
Comment 19 aaronr 2006-07-10 12:48:29 PDT
(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 :=)
Comment 20 alexander :surkov 2006-07-12 00:54:48 PDT
Created attachment 228924 [details] [diff] [review]
patch2

(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.
Comment 21 aaronr 2006-07-12 09:04:53 PDT
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.
Comment 22 alexander :surkov 2006-07-12 10:04:37 PDT
(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.
Comment 23 aaronr 2006-07-18 13:37:29 PDT
checked into trunk for surkov
Comment 24 aaronr 2006-10-17 14:18:43 PDT
checked into 1.8.0 branch on 2006/09/21
Comment 25 aaronr 2007-01-11 15:43:55 PST
checked into 1.8 branch on 2006/11/21

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