370487 - fix accesskey UI behavior for input, trigger, upload

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

checkbox should be checked/unchecked on accesskey
button should be activated on accesskey

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch2

I forgot to update guid

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Could you upload a testcase?

Comment 3

[alexander :surkov (:asurkov)]

Attachment: testcase

Comment 4

[alexander :surkov (:asurkov)]

Attachment: xul testcase

Comment 5

[aaronr]

Comment on attachment 255199 [details] [diff] [review]
patch2

You need to send a click to the anonymous input control for xf:input.  At least the html:input onclick handler gets hit for type="text".  I assume the same will be true for textarea and secret.

Also looks like you need to send a click for the upload, too.  html:input type="file" gets both click and DOMActivate passing thru it (though I assume the target of the event is actually the button -> please verify)

>Index: extensions/xforms/nsXFormsDelegateStub.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsDelegateStub.cpp,v
>retrieving revision 1.17
>diff -u -8 -p -r1.17 nsXFormsDelegateStub.cpp
>--- extensions/xforms/nsXFormsDelegateStub.cpp	13 Feb 2007 15:02:57 -0000	1.17
>+++ extensions/xforms/nsXFormsDelegateStub.cpp	15 Feb 2007 10:06:30 -0000
>@@ -82,17 +82,17 @@ nsXFormsDelegateStub::GetAccesskeyNode(n
> }
> 
> NS_IMETHODIMP
> nsXFormsDelegateStub::PerformAccesskey()
> {
>   nsCOMPtr<nsIXFormsUIWidget> widget(do_QueryInterface(mElement));
>   if (widget) {
>     PRBool isFocused;
>-    widget->Focus(&isFocused);
>+    widget->PerformAccesskey();

nit: please remove isFocused variable if not used anymore.

the rest of the stuff looks ok.  r-'ing for now just so that I get to look at the new patch.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> (From update of attachment 255199 [details] [diff] [review])
> You need to send a click to the anonymous input control for xf:input.  At least
> the html:input onclick handler gets hit for type="text".  I assume the same
> will be true for textarea and secret.
> 
> Also looks like you need to send a click for the upload, too.  html:input
> type="file" gets both click and DOMActivate passing thru it (though I assume
> the target of the event is actually the button -> please verify)

But why do we want to be compatible with events behaviour of XHTML controls?

Comment 7

[aaronr]

(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 255199 [details] [diff] [review] [details])
> > You need to send a click to the anonymous input control for xf:input.  At least
> > the html:input onclick handler gets hit for type="text".  I assume the same
> > will be true for textarea and secret.
> > 
> > Also looks like you need to send a click for the upload, too.  html:input
> > type="file" gets both click and DOMActivate passing thru it (though I assume
> > the target of the event is actually the button -> please verify)
> 
> But why do we want to be compatible with events behaviour of XHTML controls?
> 
Our accesskey support should behave consistently with the established behavior, I think.  For example, if in html an author can use a DOMActivate handler to do some kind of work when someone clicks on a pushbutton, tabs to the pushbutton and hits the spacebar or uses the alt+shift+accesskey, then I feel the author's handler should work equally well on XForms.  Otherwise there will be a hole where the user could activate a control and and the author's handler won't get called.

And this shouldn't be just a xhtml thing.  XUL probably has the same behaviors, too.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 255199 [details] [diff] [review]
patch2

I think I agree with Aaron, so clearing review request for now.

Comment 9

[alexander :surkov (:asurkov)]

I am not agree because:

1) XForms author shouldn't listen any events from anonymous content. It's our private implementation and author shouldn't do any assumption about it.
2) XUL and XHTML behaviour are quite different. Moreover behaviour for XHTML controls depends on user preferences. Ok if we will exactly copy behavour then who will observe if this behaviour will be eventually changed?
3) XForms is different language than XHTML or XUL, therefore I can assume on accesskey activation we may fire those events that are defined by XForms spec but there should not be any additional events for anonymous content because accesskey activates actually XForms control rather than anonymous content.

Comment 10

[alexander :surkov (:asurkov)]

Comment on attachment 255199 [details] [diff] [review]
patch2

reasking for review

Comment 11

[aaronr]

I guess the issue for me is that by using accesskey (a host language attribute) on a XForms control, the author shouldn't be limited to less than they normally get from the host language when they use that attribute.  I would agree with Alex's side of the argument if and only if the current xhtml behavior isn't on purpose, but rather a side effect of the combination of calls that xhtml is using.  Which, of course, might change without notice.  On the other hand, if Mozilla PURPOSELY generates these events so that a html form author can catch and handle them, then I'd say that we need to do the same.  And if they are purposely dispatching these events now, then the behavior isn't likely to change.  Accesskey is so loosely defined in the html spec to begin with, I'd prefer to give the author as consistent a behavior as possible.

Alex, can you please find someone from Mozilla to comment on the bug to let use know one way or the other?  What events, if any, are generated on purpose for the various host-language controls when accessed via accesskey?  Removing the review request until we have more information.

Comment 12

[alexander :surkov (:asurkov)]

Mats or AaronL can comment this I guess.

But I can't understand one thing. We activate XForms elments via accesskey. We do not activate underlying XHTML or XUL elements actually and therefore why do we should talk about events for anonymous XHTML or XUL content? If we will do it then it looks XUL textbox should care to fire consistent events for underlying XHTML input element. But XUL textbox doesn't do it, IIRC it just calls focus() on XUL textbox.

Comment 13

[aaronr]

(In reply to comment #12)
> Mats or AaronL can comment this I guess.
> 
> But I can't understand one thing. We activate XForms elments via accesskey. We
> do not activate underlying XHTML or XUL elements actually and therefore why do
> we should talk about events for anonymous XHTML or XUL content? If we will do
> it then it looks XUL textbox should care to fire consistent events for
> underlying XHTML input element. But XUL textbox doesn't do it, IIRC it just
> calls focus() on XUL textbox.
> 

I'm not concerned about the underlying content specifically.  What I am saying is that if an author can put a DOMActivate, click or focus handler on a control in the host language (for example a html:select) and have the handler get hit when the control gets activated via an accesskey, then if we have an analogous control when hosted in that host language, a DOMActivate or focus handler observing our control should also be hit.  Otherwise the form author is losing functionality by using XForms controls in his host document.

Comment 14

[alexander :surkov (:asurkov)]

I can see your point but still I'm not sure I'm going with you. I think we should fire those events that are defined by XForms spec. I'd rather rise a question on w3c than to bring Mozilla implementation behavior for XHTML or XUL to XForms. Because it involves xforms pages won't be compatible between different XForms proccessros.

Comment 15

[alexander :surkov (:asurkov)]

Comment on attachment 255199 [details] [diff] [review]
patch2

I guess we can deal with event stuff in another bug. Here I try to add behaviour compatible with XHTML/XUL from point of view UI.

Comment 16

[alexander :surkov (:asurkov)]

checked in on trunk

Comment 17

[alexander :surkov (:asurkov)]

I filed bug 375120 to move discussion about events on accesskey activating there.

Comment 18

[aaronr]

I'm removing xf-to-branch from this bug since it depends on the changes from bug 339287 (specifically PerformAccesskey added to interfaces) which hasn't made it to the branches, yet.  If it ever does make it to the branches, then this patch should work on the branches no problem.