1825105 - Add support for other Yahoo sites compatible with PiP Yahoo site-specific wrapper

Status: RESOLVED FIXED

(Toolkit :: Picture-in-Picture, enhancement)

Description

[kpatenio]

It seems that various other Yahoo sites use the same video player as Yahoo Finance, and therefore can be supported by our existing Yahoo wrapper.

Example sites:

• Yahoo Entertainment
• Yahoo Sports
• Yahoo News
• Yahoo Life

Adding them to our overrides will ensure that captions appear on the PiP window for these sites.

Comment 1

[Simar]

Should the overrides be added in one PR or four different PRs (one for each site)?

Comment 2

[kpatenio]

(In reply to Simar from comment #1)

Should the overrides be added in one PR or four different PRs (one for each site)?

Hi Simar, the overrides can be added in one PR. Actually, it would be easier to create overrides for all four sites under a single yahoo entry in the overrides file by renaming yahoofinance to yahoo and then adding the appropriate URLs as keys.

Comment 3

[Joseph Webster]

Hello. I’m a new contributor, and I’ll be submitting a patch for this bug soon.

Comment 4

[Joseph Webster]

Attachment: WIP: Bug 1825105

https://bugzilla.mozilla.org/show_bug.cgi?id=1825105

Comment 5

[Joseph Webster]

Hi Katherine, are there any tests I should run for this bug?

Comment 6

[Joseph Webster]

Attachment: Bug 1825105 - Add support for other Yahoo sites compatible with PiP Yahoo site-specific wrapper. r=kpatenio

Comment 7

[kpatenio]

(In reply to Joseph Webster from comment #5)

Hi Katherine, are there any tests I should run for this bug?

Hi Joseph, apologies for the delay here. Cross-posting my response from Phabricator:

We don't really have tests for wrappers, so the only way you can verify the change is by going to the sites listed in the bug description with your development build of Firefox and see if video captions appear on the PiP window. If you'd still like to run all our Picture-in-Picture tests anyways, you can use the command ./mach test toolkit/components/pictureinpicture/tests.

I hope that helps. Please reach out if you have any more questions about the bug.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e5ab0de0a58
Add support for other Yahoo sites compatible with PiP Yahoo site-specific wrapper. r=pip-reviewers,niklas

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6e5ab0de0a58

Comment 10

[Dianna Smith [:diannaS]]

:kpatenio is this something you want to nominate for 127 release notes?

Comment 11

[kpatenio]

Because of the current nature of PiP site-specific wrappers, there's a likelihood that any of the Yahoo sites will update by 127 and risk breaking this bug fix. To be safe, I'd say no, let's not nominate it for release notes.