Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-3532 keep webview wireframe hidden #1949

Conversation

xgouchet
Copy link
Collaborator

What does this PR do?

In order to keep webviews consistent in session replays, instead of removing them from the segments when computing a diff, we keep them and mark them as hidden.

@xgouchet xgouchet requested review from a team as code owners March 28, 2024 08:24
@codecov-commenter
Copy link

codecov-commenter commented Mar 28, 2024

Codecov Report

Merging #1949 (764ebdf) into feature/sr-web-view-support (2027b2d) will decrease coverage by 0.03%.
Report is 16 commits behind head on feature/sr-web-view-support.
The diff coverage is 90.91%.

Additional details and impacted files
@@                       Coverage Diff                       @@
##           feature/sr-web-view-support    #1949      +/-   ##
===============================================================
- Coverage                        83.56%   83.53%   -0.03%     
===============================================================
  Files                              486      487       +1     
  Lines                            17439    17674     +235     
  Branches                          2594     2646      +52     
===============================================================
+ Hits                             14572    14763     +191     
- Misses                            2143     2172      +29     
- Partials                           724      739      +15     
Files Coverage Δ
...internal/recorder/mapper/WebViewWireframeMapper.kt 100.00% <100.00%> (ø)
...ssionreplay/internal/processor/MutationResolver.kt 89.41% <88.89%> (-0.07%) ⬇️

... and 27 files with indirect coverage changes

android:title="@string/webview_hide"
android:id="@+id/webview_hide"
/>
</menu>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
</menu>
</menu>

Comment on lines 196 to 201
ActivityViewTrackingStrategy(true)
// NavigationViewTrackingStrategy(
// R.id.nav_host_fragment,
// true,
// SampleNavigationPredicate()
// )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is for the test or permanent change?

ARG_PAGE_NAME to "Fragment C",
ARG_WEB_VIEW_URL to
"https://datadoghq.dev/browser-sdk-test-playground/" +
"?client_token=${BuildConfig.DD_CLIENT_TOKEN}" +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably we don't want the client token to be recorded here, application ID as well. Should we use the URL from FragmentA/FragmentB?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same code as in the webview fragment, and the client token is necessary for this page.
Also the goal is to have different content to ensure the replay works (if all fragments load the same page it's hard to make sure each webview is handled independently and consistently)

"https://datadoghq.dev/browser-sdk-test-playground/" +
"?client_token=${BuildConfig.DD_CLIENT_TOKEN}" +
"&application_id=${BuildConfig.DD_RUM_APPLICATION_ID}" +
"&site=datadoghq.com"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have here a site used by the app/SDK? the one from BuildConfig.DD_SITE_NAME

@xgouchet xgouchet requested a review from 0xnm March 29, 2024 14:59
@xgouchet xgouchet merged commit 04186b5 into feature/sr-web-view-support Mar 29, 2024
23 checks passed
@xgouchet xgouchet deleted the xgouchet/RUM-3532/keep_webview_wireframe_hidden branch March 29, 2024 15:47
@xgouchet xgouchet added this to the 2.8.x milestone Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants