Closed Bug 540285 Opened 10 years ago Closed 10 years ago

stop explicit usage of nsAccEvent::prepareEvent

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #422159 - Flags: review?(marco.zehe)
Attachment #422159 - Flags: review?(ginn.chen)
Attachment #422159 - Flags: review?(bolterbugz)
try server build will be available here - https://build.mozilla.org/tryserver-builds/surkov.alexander@gmail.com-try-0a3c871f912b/ (includes 1-6 patches pointed in bug 515141).
Comment on attachment 422159 [details] [diff] [review]
patch

r=me for the test part.
Attachment #422159 - Flags: review?(marco.zehe) → review+
Comment on attachment 422159 [details] [diff] [review]
patch

r=me, overall this is an improvement thanks; please rev CID's as needed, including:

>diff --git a/accessible/src/base/nsDocAccessible.h b/accessible/src/base/nsDocAccessible.h
>--- a/accessible/src/base/nsDocAccessible.h
>+++ b/accessible/src/base/nsDocAccessible.h

please rev NS_DOCACCESSIBLE_IMPL_CID

>--- a/accessible/src/base/nsRootAccessible.h
>+++ b/accessible/src/base/nsRootAccessible.h
>@@ -103,17 +103,18 @@ public:
>       * @param aFocusEvent       DOM focus event that caused the node/accessible to receive focus
>       * @param aForceEvent       Fire a focus event even if the last focused item was the same
>       * @return                  Boolean -- was a focus event actually fired
>       */
>     PRBool FireAccessibleFocusEvent(nsIAccessible *aFocusAccessible,
>                                     nsIDOMNode *aFocusNode,
>                                     nsIDOMEvent *aFocusEvent,
>                                     PRBool aForceEvent = PR_FALSE,
>-                                    PRBool aIsAsynch = PR_FALSE);
>+                                    PRBool aIsAsynch = PR_FALSE,
>+                                    EIsFromUserInput aIsFromUserInput = eAutoDetect);

rev NS_ROOTACCESSIBLE_IMPL_CID

The event test org looks ok.
Attachment #422159 - Flags: review?(bolterbugz) → review+
Attached patch patch2Splinter Review
David comments are addressed
Attachment #422159 - Attachment is obsolete: true
Attachment #422286 - Flags: review?(ginn.chen)
Attachment #422159 - Flags: review?(ginn.chen)
// eAutoDetect: the value should be obtained from event state mangager

manager

      //    subtree or the same node, only the umbrelle event on the ancestor

umbrella


+// Constants used to point whether the event is from user input.
+enum EIsFromUserInput
+{
+  // eAutoDetect: the value should be obtained from event state mangager
+  eAutoDetect = -1,
+  // eNoUserInput: event is not from user input
+  eNoUserInput = 0,
+  // eFromUserInput: event is from user input
+  eFromUserInput = 1
+};

I don't like -1, 0, 1. It may cause trouble if we put it into bitfield.
Can we use 2, 0, 1?

+  if (aIsFromUserInput != eAutoDetect) {
+    mIsFromUserInput = static_cast<PRBool>(aIsFromUserInput);
+    return;
+  }
Please add a not in enum EIsFromUserInput declaration, in case we forget and change the value of  eNoUserInput and eFromUserInput in the future.
Or we can just use switch-case here.
Attachment #422286 - Flags: review?(ginn.chen) → review+
Ginn's comments are addressed. Landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/7fd680a94f25
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.