Closed
Bug 1445261
Opened 7 years ago
Closed 7 years ago
Replace promiseTopicObserved with TestUtils.topicObserved in browser_popupNotification_no_anchors.js
Categories
(Toolkit Graveyard :: Notifications and Alerts, enhancement, P5)
Toolkit Graveyard
Notifications and Alerts
Tracking
(firefox62 fixed)
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: johannh, Assigned: alexandersonone, Mentored)
Details
(Keywords: good-first-bug)
Attachments
(1 file)
This is a good first bug for newcomers to Firefox development.
promiseTopicObserved in the browser_popupNotification_no_anchors.js test file can be replaced by the TestUtils.topicObserved[0] utility function.
We need to replace this occurrence: https://searchfox.org/mozilla-central/rev/99df6dad057b94c2ac4d8b12f7b0b3a7fdf25d04/browser/base/content/test/popupNotifications/browser_popupNotification_no_anchors.js#170
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 browser/base/content/test/popupNotifications/browser_popupNotification_no_anchors.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.
[0] https://searchfox.org/mozilla-central/rev/588d8120aa11738657da93e09a03378bcd1ba8ec/testing/modules/TestUtils.jsm#56
Comment 1•7 years ago
|
||
Hi. I would like to work on this issue. Can i get this assigned?
Reporter | ||
Comment 2•7 years ago
|
||
Yup, thanks!
Assignee: nobody → sushma.siddamsetty
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•7 years ago
|
||
Hi, are you still interested in working on this bug? Do you need any help? :)
Flags: needinfo?(sushma.siddamsetty)
Comment 4•7 years ago
|
||
Hi,there were unsolvable issues with my system in setting up Firefox. Sorry,can't work on this.
Flags: needinfo?(sushma.siddamsetty)
Reporter | ||
Comment 5•7 years ago
|
||
(In reply to Sushma Siddamsetty from comment #4)
> Hi,there were unsolvable issues with my system in setting up Firefox.
> Sorry,can't work on this.
Oh, ok. I'm sure we can solve any issue you encounter, but I understand if you don't want to spend more time on it.
Thanks!
Assignee: sushma.siddamsetty → nobody
Status: ASSIGNED → NEW
Comment 6•7 years ago
|
||
Hi Johann! I would like to work on this issue. Can I be assigned to it?
Reporter | ||
Comment 7•7 years ago
|
||
Hi there, of course, thank you!
Assignee: nobody → vraghubir0418
Status: NEW → ASSIGNED
Comment 8•7 years ago
|
||
Hi Johann. I am having issues with setting up the mozilla development environment on Windows. I have successfully installed Visual Studio 2017,Rust and I successfully ran the hg clone https://hg.mozilla.org/mozilla-central command. The ./mach bootstrap command runs smoothly but the ./mach/build command fails for me. Any recommendations for how to fix this build issue?
Reporter | ||
Comment 9•7 years ago
|
||
(In reply to vraghubir0418 from comment #8)
> Hi Johann. I am having issues with setting up the mozilla development
> environment on Windows. I have successfully installed Visual Studio
> 2017,Rust and I successfully ran the hg clone
> https://hg.mozilla.org/mozilla-central command. The ./mach bootstrap command
> runs smoothly but the ./mach/build command fails for me. Any recommendations
> for how to fix this build issue?
Hey, sorry for the slow reply, I was away during public holiday over here. What does your ./mach build command say when it fails?
Reporter | ||
Comment 10•7 years ago
|
||
Also, feel free to contact me on IRC (https://wiki.mozilla.org/IRC) if you like.
Reporter | ||
Comment 11•7 years ago
|
||
Are you still working on this? Feel free to ask for help if you need any!
Flags: needinfo?(vraghubir0418)
Comment 12•7 years ago
|
||
Unfortunately I could not get Mozilla to build on my laptop. Sorry for the inconvenience I have caused you. Could you please unassigned me from this bug?
Flags: needinfo?(vraghubir0418)
Reporter | ||
Comment 13•7 years ago
|
||
Sure, no worries.
Assignee: vraghubir0418 → nobody
Status: ASSIGNED → NEW
Assignee | ||
Comment 14•7 years ago
|
||
I'd like to work on this bug! I am working with :sfoster
Reporter | ||
Comment 15•7 years ago
|
||
That works for me, have fun :)
Assignee: nobody → alexandersonone
Mentor: jhofmann, prathikshaprasadsuman → sfoster
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8979321 [details]
Bug 1445261 - Change promiseTopicObserved with TestUtils.topicObserved for one occurrence
https://reviewboard.mozilla.org/r/245500/#review251602
Looks good. You just need to fix up the commit message. Can you also confirm you were able to run the test cleanly? And flag johannh for review by adding r?johannh to your commit message.
::: commit-message-ca13d:1
(Diff revision 1)
> +Bug 1445261 - Change promiseTopicObserved with testUtils.topObserved for
Typo - should be TestUtils.topicObserved
Assignee | ||
Comment 18•7 years ago
|
||
Yes, the test runs properly and outputs that the tests run successfully.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8979321 [details]
Bug 1445261 - Change promiseTopicObserved with TestUtils.topicObserved for one occurrence
https://reviewboard.mozilla.org/r/245500/#review251722
This seems good, but if I'm not mistaken we can also remove the function definition now:
https://searchfox.org/mozilla-central/rev/2aa42f2cab4a110edf21dd7281ac23a1ea8901f9/browser/base/content/test/popupNotifications/head.js#16
Since we're removing the last consumer.
Thanks!
Attachment #8979321 -
Flags: review?(jhofmann)
Assignee | ||
Comment 23•7 years ago
|
||
I believe there are other uses in other files too relying on the same definition, should I delete those too?
https://searchfox.org/mozilla-central/search?q=promiseTopicObserved&path=
Assignee | ||
Comment 24•7 years ago
|
||
Nevermind, I just saw that there are no other in this folder
Comment hidden (mozreview-request) |
Reporter | ||
Comment 26•7 years ago
|
||
mozreview-review |
Comment on attachment 8979321 [details]
Bug 1445261 - Change promiseTopicObserved with TestUtils.topicObserved for one occurrence
https://reviewboard.mozilla.org/r/245500/#review252144
Looks great, thank you!
Attachment #8979321 -
Flags: review?(jhofmann) → review+
Comment 27•7 years ago
|
||
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71dbe00b841a
Change promiseTopicObserved with TestUtils.topicObserved for one occurrence r=johannh
Comment 28•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•2 years ago
|
Product: Toolkit → Toolkit Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•