1586305 - Remove workaround in browser_showMessage.js Picture-in-Picture test now that bug 1535354 is fixed

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, task, P3)

Description

[Mike Conley (:mconley) (:⚙️)]

The workaround is here:

https://searchfox.org/mozilla-central/source/toolkit/components/pictureinpicture/tests/browser_showMessage.js#37-42

this can just be removed now that bug 1535354 is fixed.

Comment 1

[Harsh]

Hi!
I would like to work on this if no one else is working on it?
Thanks

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Harsh from comment #1)

Hi!
I would like to work on this if no one else is working on it?
Thanks

Yes, you may proceed to work on this. Please ask any questions you may have here in the bug.

Comment 3

[Harsh]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)

(In reply to Harsh from comment #1)

Hi!
I would like to work on this if no one else is working on it?
Thanks

Yes, you may proceed to work on this. Please ask any questions you may have here in the bug.

I have built firefox on my machine. I just wanted to make sure that all I have to do is remove the said lines and submit a patch? This is my first time on solving bugs on this platform.
Thanks

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Harsh from comment #3)

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)

(In reply to Harsh from comment #1)

Hi!
I would like to work on this if no one else is working on it?
Thanks

Yes, you may proceed to work on this. Please ask any questions you may have here in the bug.

I have built firefox on my machine. I just wanted to make sure that all I have to do is remove the said lines and submit a patch? This is my first time on solving bugs on this platform.
Thanks

Now that you have Firefox built, you should run the test before making any changes, confirm that the test passes, then make the necessary change, run the test and confirm that the test still passes.

To run this test, you should run:
mach test toolkit/components/pictureinpicture/tests/browser_showMessage.js

Comment 5

[Harsh]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)

(In reply to Harsh from comment #3)

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)

(In reply to Harsh from comment #1)

Hi!
I would like to work on this if no one else is working on it?
Thanks

Yes, you may proceed to work on this. Please ask any questions you may have here in the bug.

I have built firefox on my machine. I just wanted to make sure that all I have to do is remove the said lines and submit a patch? This is my first time on solving bugs on this platform.
Thanks

Now that you have Firefox built, you should run the test before making any changes, confirm that the test passes, then make the necessary change, run the test and confirm that the test still passes.

To run this test, you should run:
mach test toolkit/components/pictureinpicture/tests/browser_showMessage.js

Sorry for the delay. Ran into some issues while setting up firefox. I have run the above said tests before and after making the changes. They both have passed.

I would like to submit a patch now. I am currently referring to this link(https://firefox-source-docs.mozilla.org/contributing/how_to_contribute_firefox.html#to-submit-a-patch) for doing this.

Please correct me if I am wrong.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Yes that link is correct. You might need to visit phabricator directly and use the Bugzilla Single Sign On first. But following the guide on installing moz-phab should cover all of that (you need to paste the phabricator api key etc).

Comment 7

[Harsh]

Attachment: Bug 1586305: Removed workaround in browser_showMessage.js Picture-in-Picture test r=[e-mail]

Comment 8

[Harsh]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)

Yes that link is correct. You might need to visit phabricator directly and use the Bugzilla Single Sign On first. But following the guide on installing moz-phab should cover all of that (you need to paste the phabricator api key etc).

I have submitted a patch for this bug.

Please let me know anything else that needs to be done.

Comment 9

[Harsh]

Attachment: Bug 1586305: Removed workaround in browser_showMessage.js and browser_thirdPartyIframe.js Picture-in-Picture test r=[e-mail]

Comment 10

[Harsh]

Attachment: Bug 1586305: Removed workaround in browser_showMessage.js and browser_thirdPartyIframe.js Picture-in-Picture test r=[e-mail]

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e9ff9a16b57
Removed workaround in browser_showMessage.js and browser_thirdPartyIframe.js Picture-in-Picture test r=[e-mail]

Comment 12

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/1e9ff9a16b57