296114 - xforms-next, xforms-previous event support

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

We need to support this part of the spec.  We can't do the navindex stuff since
that is in XHTML2.  We'll have to handle that when XHTML2 support enters the
browser.

Comment 1

[aaronr]

Attachment: first attempt

first attempt.	Might be better to use blur and focus if we can figure out if
the blur or focus event came from a key event as opposed to the tab or
shift+tab like I am currently doing.  That would cover cases where focus
changed by the keyboard other than using tab navigation (like via accelerator
keys).	I will check on that when I get back.  Just need to make sure we don't
generate xforms-next and xforms-previous due to mouse events changing the
focus.

Comment 2

[aaronr]

Attachment: last attempt?

never mind.  don't need to worry about the accesskey stuff for xforms-previous
and next since that might not mean the next or previous control at all. 
formsPlayer doesn't worry about it, either.

Updated patch to latest level of code.

Comment 3

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

>>>>  #xforms
smaug: should something happen if previous/next event is dispatched manually
I think we should focus previous/next control in that case
Hmm, "Navigation is determined on a containing document-wide basis. The host
language is responsible for defining overall navigation order."

Cartman: from what I can tell reading the spec, xforms-next and previous does
stuff when combined with @navindex. But navindex is a xhtml2 so ignorable for
now that is my take on it, at least

smaug: What does formsPlayer do with previous/next events , when they are
manually dispatched

Cartman: I'll see

mdubinko: It should be independent from @navindex i.e. there exists some
navigation order no matter what

Comment 4

[aaronr]

Attachment: testcase that dispatches xforms-next

This testcase, when the trigger is clicked, sets focus to the first input
control and then dispatches a xforms-next to it so that the focus should end up
on the second input control.

Comment 5

[aaronr]

Attachment: another try

this fix allows for form author to bump focus to next focusable item via
xforms-next and back an item via xforms-previous

Comment 6

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

Comment on attachment 185846 [details] [diff] [review]
another try


>+      nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
>+      nsCOMPtr<nsPIDOMWindow> win(do_QueryInterface(
>+                                               doc->GetScriptGlobalObject()));

Use the same style as above, i.e.

	nsCOMPtr<nsPIDOMWindow> win = 
	  do_QueryInterface(doc->GetScriptGlobalObject()));

Comment 7

[Doron Rosenberg (IBM)]

Comment on attachment 185846 [details] [diff] [review]
another try

>Index: nsXFormsControlStub.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsControlStub.cpp,v
>retrieving revision 1.24
>diff -u -8 -p -r1.24 nsXFormsControlStub.cpp
>--- nsXFormsControlStub.cpp	8 Jun 2005 15:33:18 -0000	1.24
>+++ nsXFormsControlStub.cpp	10 Jun 2005 07:20:15 -0000
>@@ -44,16 +44,19 @@
sEventsEntries[eEvent_Focus].name)) {
>       TryFocus(aHandled);
>+    } else if (type.Equals(NS_LITERAL_STRING("keypress"))) { 
>+      nsCOMPtr<nsIDOMKeyEvent> keyEvent = do_QueryInterface(aEvent);
>+      if (keyEvent) {
>+        PRUint32 keycode;
>+        keyEvent->GetKeyCode(&keycode);
>+        if (keycode == nsIDOMKeyEvent::DOM_VK_TAB) {
>+          PRBool extraKey = PR_FALSE;
>+          keyEvent->GetAltKey(&extraKey);
>+          if (extraKey) {
>+            return NS_OK;
>+          }
>+          keyEvent->GetCtrlKey(&extraKey);
>+          if (extraKey) {
>+            return NS_OK;
>+          }
>+          keyEvent->GetMetaKey(&extraKey);
>+          if (extraKey) {
>+            return NS_OK;
>+          }
>+

Nit: put a empty newline between each keyEvent->/if check to make it more
readable
>+      nsCOMPtr<nsIDOMDocument> domDoc;
>+      mElement->GetOwnerDocument(getter_AddRefs(domDoc));
>+
>+      nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
>+      nsCOMPtr<nsPIDOMWindow> win(do_QueryInterface(
>+                                               doc->GetScriptGlobalObject()));

do you need a check if doc is not null here?

Comment 8

[aaronr]

(In reply to comment #7)
> (From update of attachment 185846 [details] [diff] [review] [edit])
> >Index: nsXFormsControlStub.cpp
> >===================================================================
> >RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsControlStub.cpp,v
> >retrieving revision 1.24
> >diff -u -8 -p -r1.24 nsXFormsControlStub.cpp
> >--- nsXFormsControlStub.cpp	8 Jun 2005 15:33:18 -0000	1.24
> >+++ nsXFormsControlStub.cpp	10 Jun 2005 07:20:15 -0000
> >@@ -44,16 +44,19 @@
> sEventsEntries[eEvent_Focus].name)) {
> >       TryFocus(aHandled);
> >+    } else if (type.Equals(NS_LITERAL_STRING("keypress"))) { 
> >+      nsCOMPtr<nsIDOMKeyEvent> keyEvent = do_QueryInterface(aEvent);
> >+      if (keyEvent) {
> >+        PRUint32 keycode;
> >+        keyEvent->GetKeyCode(&keycode);
> >+        if (keycode == nsIDOMKeyEvent::DOM_VK_TAB) {
> >+          PRBool extraKey = PR_FALSE;
> >+          keyEvent->GetAltKey(&extraKey);
> >+          if (extraKey) {
> >+            return NS_OK;
> >+          }
> >+          keyEvent->GetCtrlKey(&extraKey);
> >+          if (extraKey) {
> >+            return NS_OK;
> >+          }
> >+          keyEvent->GetMetaKey(&extraKey);
> >+          if (extraKey) {
> >+            return NS_OK;
> >+          }
> >+
> 
> Nit: put a empty newline between each keyEvent->/if check to make it more
> readable
> >+      nsCOMPtr<nsIDOMDocument> domDoc;
> >+      mElement->GetOwnerDocument(getter_AddRefs(domDoc));
> >+
> >+      nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
> >+      nsCOMPtr<nsPIDOMWindow> win(do_QueryInterface(
> >+                                               doc->GetScriptGlobalObject()));
> 
> do you need a check if doc is not null here?
> 

I didn't think that I'd need to check for an owner document considering that
xforms-next is either generated due to a tab or shift+tab to the control or
dispatched to it and I didn't see how either could be done without the control
being in a document.  I'll add a check to be safe.

Comment 9

[aaronr]

Attachment: fixed nits

fixes smaug's and doron's nits

Comment 10

[Doron Rosenberg (IBM)]

checked in

Comment 11

[Allan Beaufour]

(In reply to comment #10)
> checked in

I allowed myself to fix the compiler warning in nsXFormsControlStub.h.