Navigate to YT from Duck Player #4780

CrisBarreiro · 2024-07-19T12:00:11Z

Task/Issue URL: https://app.asana.com/0/0/1207562793310532/f

Description

Steps to test this PR

Feature 1

[ ]
[ ]

UI changes

Before	After
!(Upload before screenshot)	(Upload after screenshot)

CrisBarreiro · 2024-07-19T12:00:32Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

Navigate to YT from Duck Player #4780 👈
Add support for Duck Player #4663 : 12 other dependent PRs (#4748 , #4752 , #4824 and 9 others)
develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @CrisBarreiro and the rest of your teammates on Graphite

marcosholgado

Overall LGTM. Not really sure what to test functionality wise since there are no testing steps and more tests could be added. Will wait to get the different scenarios to test before approving.

marcosholgado · 2024-08-08T14:44:41Z

app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt

@@ -1092,7 +1092,7 @@ class BrowserTabViewModel @Inject constructor(
        when (stateChange) {
            is WebNavigationStateChange.NewPage -> {
                val uri = stateChange.url.toUri()
-                if (duckPlayer.isYoutubeNoCookie(uri)) {
+                if (duckPlayer.isSimulatedYoutubeNoCookie(uri)) {


All the changes to the view model should probably be tested.

Updated in top of the stack

marcosholgado · 2024-08-08T14:45:24Z

app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt

@@ -383,12 +383,15 @@ class BrowserWebViewClient @Inject constructor(
                        pageLoadedHandler.onPageLoaded(it, navigationList.currentItem?.title, safeStart, currentTimeProvider.elapsedRealtime())
                        shouldSendPagePaintedPixel(webView = webView, url = it)
                        appCoroutineScope.launch(dispatcherProvider.io()) {
-                            if (duckPlayer.isYoutubeNoCookie(url)) {
+                            if (duckPlayer.isSimulatedYoutubeNoCookie(url)) {


Updated in top of the stack

marcosholgado · 2024-08-08T14:46:40Z

app/src/main/java/com/duckduckgo/app/browser/WebViewRequestInterceptor.kt

@@ -159,6 +122,10 @@ class WebViewRequestInterceptor(
            return WebResourceResponse(null, null, null)
        }

+        if (url != null) {


Added to top of the stack

marcosholgado · 2024-08-08T14:47:16Z

duckplayer/duckplayer-api/build.gradle

@@ -34,6 +34,7 @@ dependencies {
    implementation Kotlin.stdlib.jdk7
    implementation AndroidX.core.ktx
    implementation KotlinX.coroutines.core
+    implementation 'androidx.legacy:legacy-support-v4:1.0.0'


Curious why is this needed.

marcosholgado · 2024-08-08T14:48:20Z

duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/RealDuckPlayer.kt


+@SingleInstanceIn(AppScope::class)
 @ContributesBinding(AppScope::class)
 class RealDuckPlayer @Inject constructor(


Tested in the top of the stack

marcosholgado · 2024-08-09T15:15:13Z

Had a go at the test cases and found these behaviours not matching them.

Back button with Always Ask

Open a video in YouTube
Go to Duck Player
On the Duck Player page use the back button on your phone
The video should load in YouTube with no overlay shown <- This doesn't happen for me and the overlay is always shown.

Back button with Always

Open a video in YouTube, check Duck Player is loaded
On the Duck Player page use the back button on your phone
The video should load in YouTube <- This never happens and I'm taken back to the new tab screen instead. Which I think makes sense because YT never actually loads.

Back button age restricted videos

Open video using Duck Player URL
On the Duck Player page use the back button on your phone
Check YouTube is loaded <- as above, YT doesn't load, instead I'm taken to the new tab screen.

CrisBarreiro · 2024-08-09T17:17:01Z

Had a go at the test cases and found these behaviours not matching them.

Back button with Always Ask

* Open a video in YouTube

* Go to Duck Player

* On the Duck Player page use the back button on your phone

* The video should load in YouTube with no overlay shown <- This doesn't happen for me and the overlay is always shown.

Back button with Always

* Open a video in YouTube, check Duck Player is loaded

* On the Duck Player page use the back button on your phone

* The video should load in YouTube <- This never happens and I'm taken back to the new tab screen instead. Which I think makes sense because YT never actually loads.

Back button age restricted videos

* Open video using Duck Player URL

* On the Duck Player page use the back button on your phone

* Check YouTube is loaded <- as above, YT doesn't load, instead I'm taken to the new tab screen.

@marcosholgado thanks for having a look!

Back button with Always Ask -> I think the current behavior makes sense, as the page the user had seen before contains the overlay, and we probably shouldn't treat it in the same way as tapping the YouTube button

For the other 2 cases, as you mentioned, since YouTube never actually loads, it doesn't make sense to navigate back to it. That was a mistake on my site when writing the test cases

marcosholgado

LGTM

This was referenced Jul 19, 2024

Add support for Duck Player #4663

Merged

Add Duck Player settings UI #4748

Closed

CrisBarreiro mentioned this pull request Jul 19, 2024

Update initial setup message for Duck Player page #4752

Merged

4 tasks

CrisBarreiro marked this pull request as ready for review July 30, 2024 09:29

CrisBarreiro marked this pull request as draft July 30, 2024 09:30

CrisBarreiro force-pushed the feature/cris/navigate-back-from-duck-player branch from b267e13 to 4a370c6 Compare July 30, 2024 09:43

CrisBarreiro mentioned this pull request Jul 31, 2024

Add Duck Player prime modal #4824

Merged

3 tasks

CrisBarreiro force-pushed the feature/cris/navigate-back-from-duck-player branch 2 times, most recently from 5172524 to 8789ab0 Compare August 5, 2024 09:42

CrisBarreiro force-pushed the feature/cris/update-initial-setup-duck-player-page branch from 40fc837 to d9fa7be Compare August 7, 2024 09:56

CrisBarreiro force-pushed the feature/cris/navigate-back-from-duck-player branch from 8789ab0 to 8c40743 Compare August 7, 2024 09:56

CrisBarreiro force-pushed the feature/cris/update-initial-setup-duck-player-page branch from d9fa7be to 8242a9f Compare August 7, 2024 10:28

CrisBarreiro force-pushed the feature/cris/navigate-back-from-duck-player branch from 8c40743 to 1865035 Compare August 7, 2024 10:28

CrisBarreiro marked this pull request as ready for review August 7, 2024 10:32

CrisBarreiro force-pushed the feature/cris/update-initial-setup-duck-player-page branch from 8242a9f to fab0899 Compare August 7, 2024 13:16

CrisBarreiro force-pushed the feature/cris/navigate-back-from-duck-player branch from 1865035 to 742ea02 Compare August 7, 2024 13:17

CrisBarreiro mentioned this pull request Aug 7, 2024

Add contingency settings #4872

Merged

CrisBarreiro force-pushed the feature/cris/update-initial-setup-duck-player-page branch from fab0899 to 8d68e61 Compare August 8, 2024 11:44

CrisBarreiro force-pushed the feature/cris/navigate-back-from-duck-player branch from 742ea02 to ae1e3be Compare August 8, 2024 11:44

CrisBarreiro force-pushed the feature/cris/update-initial-setup-duck-player-page branch from 8d68e61 to 1f34224 Compare August 8, 2024 11:50

CrisBarreiro force-pushed the feature/cris/navigate-back-from-duck-player branch from ae1e3be to 54cc443 Compare August 8, 2024 11:50

marcosholgado reviewed Aug 8, 2024

View reviewed changes

marcosholgado self-assigned this Aug 9, 2024

CrisBarreiro mentioned this pull request Aug 14, 2024

Update initialSetup with allowFirstVideo #4897

Closed

3 tasks

marcosholgado approved these changes Aug 14, 2024

View reviewed changes

This was referenced Aug 14, 2024

Add pixels #4902

Closed

Create RMF matchers for duckPlayerOnboarded and duckPlayerEnabled #4917

Closed

CrisBarreiro mentioned this pull request Aug 19, 2024

Request translations #4919

Merged

1 task

CrisBarreiro force-pushed the feature/cris/navigate-back-from-duck-player branch 2 times, most recently from 0a98f10 to 2d3b27f Compare August 20, 2024 11:14

Navigate back to YouTube from Duck Player

9539c9c

CrisBarreiro force-pushed the feature/cris/update-initial-setup-duck-player-page branch from 1f34224 to 878ad02 Compare August 20, 2024 15:28

CrisBarreiro force-pushed the feature/cris/navigate-back-from-duck-player branch from 2d3b27f to 9539c9c Compare August 20, 2024 15:28

Base automatically changed from feature/cris/update-initial-setup-duck-player-page to feature/cris/duckplayer/create-module August 20, 2024 15:37

CrisBarreiro merged commit 9539c9c into feature/cris/duckplayer/create-module Aug 20, 2024
4 of 5 checks passed

CrisBarreiro deleted the feature/cris/navigate-back-from-duck-player branch August 20, 2024 15:40

This was referenced Aug 21, 2024

Hide onboarding when Duck Player page or overlay are shown #4924

Closed

Update Duck Player copy #4931

Merged

Fire pixel when skipping onboarding #4943

Merged

CrisBarreiro mentioned this pull request Aug 28, 2024

Replace "Always Ask" with "Ask Every Time" #4947

Merged

This was referenced Sep 10, 2024

Add support for rollout #4996

Merged

Add DuckPlayer web files to VCS #4999

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Navigate to YT from Duck Player #4780

Navigate to YT from Duck Player #4780

CrisBarreiro commented Jul 19, 2024

CrisBarreiro commented Jul 19, 2024 •

edited

Loading

marcosholgado left a comment

marcosholgado Aug 8, 2024

CrisBarreiro Aug 12, 2024

marcosholgado Aug 8, 2024

CrisBarreiro Aug 12, 2024

marcosholgado Aug 8, 2024

CrisBarreiro Aug 12, 2024

marcosholgado Aug 8, 2024

marcosholgado Aug 8, 2024

CrisBarreiro Aug 12, 2024

marcosholgado commented Aug 9, 2024

CrisBarreiro commented Aug 9, 2024

marcosholgado left a comment

Navigate to YT from Duck Player #4780

Navigate to YT from Duck Player #4780

Conversation

CrisBarreiro commented Jul 19, 2024

Description

Steps to test this PR

UI changes

CrisBarreiro commented Jul 19, 2024 • edited Loading

marcosholgado left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosholgado commented Aug 9, 2024

CrisBarreiro commented Aug 9, 2024

marcosholgado left a comment

Choose a reason for hiding this comment

CrisBarreiro commented Jul 19, 2024 •

edited

Loading