Replace promiseWaitForCondition with TestUtils.waitForCondition in browser/base/content/test/webrtc/head.js

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P5
normal
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: johannh, Assigned: carolina.jimenez.g, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 67
Points:
---

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(1 attachment)

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

promiseWaitForCondition in browser/base/content/test/webrtc/head.js can be replaced by the TestUtils.waitForCondition utility function.

The code in question is here: https://searchfox.org/mozilla-central/rev/dc0adc07db3df9431a0876156f50c65d580010cb/browser/base/content/test/webrtc/head.js#96

You can also remove the function definition: https://searchfox.org/mozilla-central/rev/dc0adc07db3df9431a0876156f50c65d580010cb/browser/base/content/test/webrtc/head.js#37

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.

After your change it might be a good idea to run all WebRTC UI tests with the ./mach mochitest command:

./mach mochitest browser/base/content/test/webrtc/

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.

Assignee

Comment 1

4 months ago

Hello, I'm an outreachy applicant, can I try to solve this bug?

Absolutely, let me know if you're stuck!

Assignee: nobody → carolina.jimenez.g
Status: NEW → ASSIGNED
Assignee

Comment 3

4 months ago

however, in the file head.js other functions use BrowserTestUtils instead of TestUtils, so I wasn't sure which of those is the
correct one, so I used the one suggested in the issue.

Assignee

Comment 4

4 months ago

Also BrowserTestUtils and TestUtils both passed the tests with the command ./mach mochitest browser/base/content/test/webrtc/

Thanks! To get this patch landed you will need to set the checkin-needed flag on the bug. This time I've done it for you :)

Keywords: checkin-needed
Assignee

Comment 6

4 months ago

Thank you! how did you added?

Assignee

Comment 7

4 months ago

how did you add it?***

I think you need editbugs privileges, which you will usually get after working on two bugs :)

Comment 9

4 months ago

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/45bc26c5c749
Change promiseWaitForCondition for TestUtils.waitForCondition r=johannh

Keywords: checkin-needed

Comment 10

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Assignee

Comment 11

4 months ago

I can be wrong but I think there are problems blocking the cookies, here are them:

  1. When a person add a url that doesn't even exist like putting just "facebook" it let you do it. I don't know if is the correct behavior.

  2. When I add facebook.com and I go to google.com it shows like if we had added google in the list of blocked cookies, but I didn't.

I would like to attach some images regarding this behavior but I don't know how to do it...

Assignee

Comment 12

4 months ago

Sorry, I commented on the wrong issue

You need to log in before you can comment on or make changes to this bug.