1530765 - Replace waitForEvent in browser_pageinfo_firstPartyIsolation.js with BrowserTestUtils.waitForEvent

Status: RESOLVED FIXED

(Firefox :: Page Info Window, enhancement, P5)

Description

[Johann Hofmann [:johannh]]

This is a good first bug for newcomers to Firefox development.

waitForEvent in the browser_pageinfo_firstPartyIsolation.js test file can be replaced by the BrowserTestUtils.waitForEvent utility function.

The code in question is here: https://searchfox.org/mozilla-central/rev/b10ae6b7a50d176a813900cbe9dc18c85acd604b/browser/base/content/test/pageinfo/browser_pageinfo_firstPartyIsolation.js#3,35,37,84

For instructions on how to get your local build of Firefox up and running and submit your patch, see https://developer.mozilla.org/en-US/docs/Introduction.

You can run this test with the ./mach mochitest command:

./mach mochitest base/content/test/pageinfo/browser_pageinfo_firstPartyIsolation.js

Please leave a comment if you would like to be assigned to this bug and feel free to ask questions here or via IRC if you're stuck.

Comment 1

[Jawad Ahmed [:jawad]]

Hi, I am willing to work on this bug, please assign it to me.

Comment 2

[Johann Hofmann [:johannh]]

Sure, thank you!

Comment 3

[Jawad Ahmed [:jawad]]

Attachment: waitForEvent in the browser_pageinfo_firstPartyIsolation.js replaced by the BrowserTestUtils.waitForEvent utility function

Comment 4

[Johann Hofmann [:johannh]]

Comment on attachment 9046902 [details] [diff] [review]
waitForEvent in the browser_pageinfo_firstPartyIsolation.js replaced by the BrowserTestUtils.waitForEvent utility function

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

Sorry if my description was unclear, the idea is to stop using the `waitForEvent` function and utilizing `BrowserTestUtils.waitForEvent` instead, so you can just remove the function definition entirely :)

Also note that you will have to start using Phabricator soon, since we will be shutting down code review via Bugzilla, I think about tomorrow.

Thanks!

Comment 5

[Jawad Ahmed [:jawad]]

Attachment: waitForEvent in the browser_pageinfo_firstPartyIsolation.js replaced by the BrowserTestUtils.waitForEvent utility function

Hi, sorry for my misunderstanding, thank you for your helpful reply.

Comment 6

[Johann Hofmann [:johannh]]

Comment on attachment 9047025 [details] [diff] [review]
waitForEvent in the browser_pageinfo_firstPartyIsolation.js replaced by the BrowserTestUtils.waitForEvent utility function

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

Looks great, thanks!

Comment 7

[Johann Hofmann [:johannh]]

Oh, one thing, please format your commit message like this:

Bug 1530765 - Replace waitForEvent in the browser_pageinfo_firstPartyIsolation.js by the BrowserTestUtils.waitForEvent utility function. r=johannh

and upload the patch again. Then we can get it landed :)

Comment 8

[Jawad Ahmed [:jawad]]

Attachment: waitForEvent in the browser_pageinfo_firstPartyIsolation.js by the BrowserTestUtils.waitForEvent utility function

Formatted commit message.

Comment 9

[Johann Hofmann [:johannh]]

Comment on attachment 9047060 [details] [diff] [review]
waitForEvent in the browser_pageinfo_firstPartyIsolation.js by the BrowserTestUtils.waitForEvent utility function

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

Thanks!

Comment 10

[Johann Hofmann [:johannh]]

Let's get this landed!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e743e4fa9a4d
Replace waitForEvent in the browser_pageinfo_firstPartyIsolation.js by the BrowserTestUtils.waitForEvent utility function. r=johannh CLOSED TREE

Comment 12

[Jawad Ahmed [:jawad]]

(In reply to Johann Hofmann [:johannh] from comment #10)

Let's get this landed!

Thank you :)

Comment 13

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/e743e4fa9a4d