Passwordmgr tests: Use EventUtils sendChar() and sendKey(), instead of calling synthesizeKey() directly

VERIFIED FIXED in mozilla12

Status

()

--
minor
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: karun.84, Assigned: karun.84)

Tracking

Trunk
mozilla12
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

7 years ago
Referencing bug Bug 720087, I plan on fixing this in the passwordmgr tests first. Ive created an initial bug for this section.
(Assignee)

Updated

7 years ago
Blocks: 720087
Assignee: nobody → karun.84
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Version: unspecified → Trunk
(Assignee)

Comment 1

7 years ago
Created attachment 591914 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with sendKey

Here is the patch for this bug for password manager. I have ran the mochitests locally, and have not encountered any errors with this patch.
(Assignee)

Comment 2

7 years ago
Created attachment 591916 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with sendKey - With code formatting corrected

I added the patch again, with the code formatting fixed. For review.
Attachment #591914 - Attachment is obsolete: true
Attachment #591916 - Flags: review?(paul)
Comment on attachment 591916 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with sendKey - With code formatting corrected

Review of attachment 591916 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/passwordmgr/test/browser/browser_passwordmgr_sort.js
@@ +113,5 @@
>                          Services.ww.unregisterNotification(arguments.callee);
>                      else if (aTopic == "domwindowopened") {
>                          let win = aSubject.QueryInterface(Ci.nsIDOMEventTarget);
>                          SimpleTest.waitForFocus(function() {
> +                            EventUtils.sendKey("VK_RETURN")

No, this should become
EventUtils.sendKey("RETURN", win)

::: toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js
@@ +123,5 @@
>                          Services.ww.unregisterNotification(arguments.callee);
>                      else if (aTopic == "domwindowopened") {
>                          let win = aSubject.QueryInterface(Ci.nsIDOMEventTarget);
>                          SimpleTest.waitForFocus(function() {
> +			    EventUtils.sendKey("VK_RETURN")

There are still tabs on your new line :-/
Attachment #591916 - Flags: review?(paul) → review-
(In reply to Karun Dambiec from comment #1)
> I have ran the mochitests locally, and have not encountered any errors with this patch.

Hum, at least with a debug build, you should have gotten something like
"JavaScript error: , line 0: uncaught exception: Unknown key: VK_VK_RETURN"
(Assignee)

Comment 5

7 years ago
(In reply to Serge Gautherie (:sgautherie) from comment #4)
> (In reply to Karun Dambiec from comment #1)
> > I have ran the mochitests locally, and have not encountered any errors with this patch.
> 
> Hum, at least with a debug build, you should have gotten something like
> "JavaScript error: , line 0: uncaught exception: Unknown key: VK_VK_RETURN"

Ok, thanks. Maybe I need to have a look at how my build is setup. I will make a new patch for this bug, with it fixed to be RETURN, and not VK_Return.
(Assignee)

Comment 6

7 years ago
Created attachment 591932 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with sendKey - With code formatting corrected and use of Sendkey corrected

I have fixed the code formatting, and the use of sendKey with this version of the patch
Attachment #591916 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Attachment #591932 - Flags: review?(sgautherie.bz)
Comment on attachment 591932 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with sendKey - With code formatting corrected and use of Sendkey corrected

Review of attachment 591932 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/passwordmgr/test/browser/browser_passwordmgr_sort.js
@@ +113,5 @@
>                          Services.ww.unregisterNotification(arguments.callee);
>                      else if (aTopic == "domwindowopened") {
>                          let win = aSubject.QueryInterface(Ci.nsIDOMEventTarget);
>                          SimpleTest.waitForFocus(function() {
> +                            EventUtils.sendKey("RETURN", win)

Nit: while here, could you try adding a ';'.
Attachment #591932 - Flags: review?(sgautherie.bz)
Attachment #591932 - Flags: review?(paul)
Attachment #591932 - Flags: feedback+
(Assignee)

Comment 8

7 years ago
(In reply to Serge Gautherie (:sgautherie) from comment #7)
> Comment on attachment 591932 [details] [diff] [review]
> Patch for this bug to replace synthesizeKey with sendKey - With code
> formatting corrected and use of Sendkey corrected
> 
> Review of attachment 591932 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/passwordmgr/test/browser/browser_passwordmgr_sort.js
> @@ +113,5 @@
> >                          Services.ww.unregisterNotification(arguments.callee);
> >                      else if (aTopic == "domwindowopened") {
> >                          let win = aSubject.QueryInterface(Ci.nsIDOMEventTarget);
> >                          SimpleTest.waitForFocus(function() {
> > +                            EventUtils.sendKey("RETURN", win)
> 
> Nit: while here, could you try adding a ';'.

Yes a ; is good. I was not sure if this was a specific coding practice for the password manager/firefox, and did not want to change it  much initially.
(Assignee)

Comment 9

7 years ago
Created attachment 591941 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with sendKey - Code Formatting fixed

This is a revision of the previous patch with a ; added to the javascript.
Attachment #591932 - Attachment is obsolete: true
Attachment #591932 - Flags: review?(paul)
Attachment #591941 - Flags: review?(paul)
Comment on attachment 591941 [details] [diff] [review]
Patch for this bug to replace synthesizeKey with sendKey - Code Formatting fixed

Review of attachment 591941 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thanks for the contribution!
Attachment #591941 - Flags: review?(paul) → review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab27cd796845
Keywords: checkin-needed
Target Milestone: --- → mozilla12
(Assignee)

Updated

7 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

7 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Updated

7 years ago
Attachment #591941 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/ab27cd796845
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
V.Fixed, per mxr search.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.