xforms-next, xforms-previous event support

RESOLVED FIXED

Status

RESOLVED FIXED
14 years ago
2 years ago

People

(Reporter: aaronr, Assigned: aaronr)

Tracking

Trunk
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

14 years ago
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.
(Assignee)

Comment 1

14 years ago
Created attachment 184947 [details] [diff] [review]
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.
(Assignee)

Comment 2

14 years ago
Created attachment 185694 [details] [diff] [review]
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.
Attachment #184947 - Attachment is obsolete: true
Attachment #185694 - Flags: review?(smaug)
>>>>  #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
(Assignee)

Comment 4

14 years ago
Created attachment 185827 [details]
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.
(Assignee)

Updated

14 years ago
Attachment #185694 - Flags: review?(smaug)
(Assignee)

Comment 5

14 years ago
Created attachment 185846 [details] [diff] [review]
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
Attachment #185694 - Attachment is obsolete: true
Attachment #185846 - Flags: review?(smaug)
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()));
Attachment #185846 - Flags: review?(smaug)
Attachment #185846 - Flags: review?(doronr)
Attachment #185846 - Flags: review+
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?
Attachment #185846 - Flags: review?(doronr) → review+
(Assignee)

Comment 8

14 years ago
(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.
(Assignee)

Comment 9

14 years ago
Created attachment 186129 [details] [diff] [review]
fixed nits

fixes smaug's and doron's nits
Attachment #185846 - Attachment is obsolete: true
checked in
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 11

14 years ago
(In reply to comment #10)
> checked in

I allowed myself to fix the compiler warning in nsXFormsControlStub.h.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.