300407 - Need to prevent default action when pressing F1 in an XForms control

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Doron Rosenberg (IBM)]

On windows, we show the mozilla product help dialog and the xforms:hint url.

Comment 1

[Doron Rosenberg (IBM)]

Attachment: le patch

Problem is, this patch will always prevent default, even if there is no
xforms:hint

Comment 2

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

(In reply to comment #1)
> Created an attachment (id=188978) [edit]
> le patch
> 
> Problem is, this patch will always prevent default, even if there is no
> xforms:hint

I guess DispatchEvent could return defaultActionEnabled as an |out| parameter.
http://lxr.mozilla.org/seamonkey/source/extensions/xforms/nsXFormsUtils.cpp#855
and then the preventDefault could be called on help event in 
http://lxr.mozilla.org/seamonkey/source/extensions/xforms/nsXFormsMessageElement.cpp#428
This way the nsIDOMKeyEvent::DOM_VK_F1 keyevent should be prevented only
if the help-event is prevented.

Comment 3

[Doron Rosenberg (IBM)]

Attachment: better patch

Follow's smaug's idea, which works!

Comment 4

[Doron Rosenberg (IBM)]

Attachment: with smaug's irc comment addressed

Comment 5

[aaronr]

Comment on attachment 189083 [details] [diff] [review]
with smaug's irc comment addressed

nits mostly.

>Index: extensions/xforms/nsXFormsControlStub.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsControlStub.cpp,v
>retrieving revision 1.27
>diff -p -w -u -1 -0 -r1.27 nsXFormsControlStub.cpp
>--- extensions/xforms/nsXFormsControlStub.cpp	29 Jun 2005 18:14:43 -0000	1.27
>+++ extensions/xforms/nsXFormsControlStub.cpp	12 Jul 2005 19:50:14 -0000
>@@ -69,22 +69,31 @@ nsXFormsHintHelpListener::HandleEvent(ns
>     return NS_ERROR_UNEXPECTED;
> 
>   nsCOMPtr<nsIDOMEventTarget> target;
>   aEvent->GetCurrentTarget(getter_AddRefs(target));
>   nsCOMPtr<nsIDOMNode> targetNode(do_QueryInterface(target));
>   if (nsXFormsUtils::EventHandlingAllowed(aEvent, targetNode)) {
>     nsCOMPtr<nsIDOMKeyEvent> keyEvent(do_QueryInterface(aEvent));
>     if (keyEvent) {
>       PRUint32 code = 0;
>       keyEvent->GetKeyCode(&code);
>-      if (code == nsIDOMKeyEvent::DOM_VK_F1)
>-        nsXFormsUtils::DispatchEvent(targetNode, eEvent_Help);
>+      if (code == nsIDOMKeyEvent::DOM_VK_F1) {
>+        PRBool defaultEnabled = PR_TRUE;
>+        nsXFormsUtils::DispatchEvent(targetNode, eEvent_Help, &defaultEnabled);
>+
>+        // If the xforms event's default behavior was disabled, prevent the DOM
>+        // event's default action as well.  This means that the F1 key was
>+        // handled (a help dialog was shown) and we don't want the browser to
>+        // show it's help dialog.
>+        if (!defaultEnabled)
>+          aEvent->PreventDefault();
>+      }

You are changing the default behavior of this event without checking the return
code from DispatchEvent.  Probably should only do this is DispatchEvent was
successful, right?

> 
>Index: extensions/xforms/nsXFormsMessageElement.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsMessageElement.cpp,v
>retrieving revision 1.8
>diff -p -w -u -1 -0 -r1.8 nsXFormsMessageElement.cpp
>--- extensions/xforms/nsXFormsMessageElement.cpp	26 Jun 2005 18:30:17 -0000	1.8
>+++ extensions/xforms/nsXFormsMessageElement.cpp	12 Jul 2005 19:50:20 -0000
>@@ -422,20 +422,21 @@ nsXFormsMessageElement::HandleAction(nsI
>     case eType_Hint:
>       level.AssignLiteral("ephemeral");
>       // Using the innermost <hint>.
>       aEvent->StopPropagation();
>       break;
>     case eType_Help:
>       level.AssignLiteral("modeless");
>       // <help> is equivalent to a
>       // <message level="modeless" ev:event="xforms-help" ev:propagate="stop>.
>       aEvent->StopPropagation();
>+      aEvent->PreventDefault();
>       break;
>     case eType_Alert:
>       if (HandleInlineAlert(aEvent))
>         return NS_OK;
> 
>       level.AssignLiteral("modal");
>       break;
>   }
> 
>   if (level.IsEmpty())
>Index: extensions/xforms/nsXFormsUtils.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsUtils.cpp,v
>retrieving revision 1.45
>diff -p -w -u -1 -0 -r1.45 nsXFormsUtils.cpp
>--- extensions/xforms/nsXFormsUtils.cpp	4 Jul 2005 16:13:59 -0000	1.45
>+++ extensions/xforms/nsXFormsUtils.cpp	12 Jul 2005 19:50:25 -0000
>@@ -786,21 +786,22 @@ nsXFormsUtils::GetSingleNodeBindingValue
>   nsCOMPtr<nsIDOMNode> singleNode;
>   result->GetSingleNodeValue(getter_AddRefs(singleNode));
>   if (!singleNode)
>     return PR_FALSE;
> 
>   nsXFormsUtils::GetNodeValue(singleNode, aValue);
>   return PR_TRUE;
> }
> 
> /* static */ nsresult
>-nsXFormsUtils::DispatchEvent(nsIDOMNode* aTarget, nsXFormsEvent aEvent)
>+nsXFormsUtils::DispatchEvent(nsIDOMNode* aTarget, nsXFormsEvent aEvent,
>+                             PRBool *aDefaultActionEnabled)
> {
>   if (!aTarget)
>     return NS_ERROR_FAILURE;
> 
>   nsCOMPtr<nsIXFormsControl> control = do_QueryInterface(aTarget);
>   if (control) {
>     switch (aEvent) {
>       case eEvent_Previous:
>       case eEvent_Next:
>       case eEvent_Focus:
>@@ -845,22 +846,27 @@ nsXFormsUtils::DispatchEvent(nsIDOMNode*
> 
>   const EventData *data = &sXFormsEventsEntries[aEvent];
>   event->InitEvent(NS_ConvertUTF8toUTF16(data->name),
>                    data->canBubble, data->canCancel);
>   
>   nsCOMPtr<nsIDOMEventTarget> target = do_QueryInterface(aTarget);
>   NS_ENSURE_STATE(target);
> 
>   SetEventTrusted(event, aTarget);
> 
>-  PRBool defaultActionEnabled;
>-  return target->DispatchEvent(event, &defaultActionEnabled);
>+  PRBool defaultActionEnabled = PR_TRUE;
>+  nsresult rv = target->DispatchEvent(event, &defaultActionEnabled);
>+
>+  if (aDefaultActionEnabled)
>+    *aDefaultActionEnabled = defaultActionEnabled;
>+
>+  return rv;
> }

You are changing the value in the return buffer even if you are returning an
error.	Ick!

> 
> /* static */ nsresult
> nsXFormsUtils::SetEventTrusted(nsIDOMEvent* aEvent, nsIDOMNode* aRelatedNode)
> {
>   nsCOMPtr<nsIDOMNSEvent> event(do_QueryInterface(aEvent));
>   if (event) {
>     PRBool isTrusted = PR_FALSE;
>     event->GetIsTrusted(&isTrusted);
>     if (!isTrusted && aRelatedNode) {
>Index: extensions/xforms/nsXFormsUtils.h
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsUtils.h,v
>retrieving revision 1.28
>diff -p -w -u -1 -0 -r1.28 nsXFormsUtils.h
>--- extensions/xforms/nsXFormsUtils.h	26 Jun 2005 18:30:17 -0000	1.28
>+++ extensions/xforms/nsXFormsUtils.h	12 Jul 2005 19:50:30 -0000
>@@ -261,21 +261,22 @@ public:
>    * for an element.
>    * Returns PR_TRUE if the evaluation succeeds.
>    */
>   static NS_HIDDEN_(PRBool)
>     GetSingleNodeBindingValue(nsIDOMElement* aElement, nsString& aValue);
> 
>   /**
>    * Dispatch an XForms event. 
>    */
>   static NS_HIDDEN_(nsresult)
>-    DispatchEvent(nsIDOMNode* aTarget, nsXFormsEvent aEvent);
>+    DispatchEvent(nsIDOMNode* aTarget, nsXFormsEvent aEvent,
>+                  PRBool *aDefaultActionEnabled = nsnull);
> 

Should probably explain what the aDefaultActionEnabled means here.  Not quite
as apparent as the other two params.

with those, r=me

Comment 6

[Doron Rosenberg (IBM)]

checked in.