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

[REPLAY] Fix serialization for checkbox & radio #2021

Merged
merged 3 commits into from
Feb 22, 2023

Conversation

ThibautGeriz
Copy link
Contributor

@ThibautGeriz ThibautGeriz commented Feb 17, 2023

Motivation

When serialise checkbox & radio with mask-input-privacy. It would record the input with checked: '***' on all the input regardless or if it was set.

It would be re-created like this in the player

<label>
  <input
    type="radio"
    name="name1"
    value="***"
    checked="***"
  />
  <span>option 1</span>
</label>
<label class="c-radio">
  <input
    type="radio"
    name="name1"
    value="***"
    checked="***"
  />
  <span>option 2</span>
</label>

Meaning all the radio & checkbox would be checked.

Let's uncheck all of them when that's the privacy setting

Changes

Delete the checked attribute instead of the overding it with ***

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2023

Codecov Report

Merging #2021 (157f74f) into main (a483e80) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2021      +/-   ##
==========================================
- Coverage   93.59%   93.48%   -0.11%     
==========================================
  Files         145      145              
  Lines        5512     5512              
  Branches     1250     1250              
==========================================
- Hits         5159     5153       -6     
- Misses        353      359       +6     
Impacted Files Coverage Δ
packages/rum/test/htmlAst.ts 92.00% <ø> (ø)
packages/rum/src/domain/record/serialize.ts 96.29% <100.00%> (ø)
...s/rum-core/src/domain/contexts/pageStateHistory.ts 80.48% <0.00%> (-12.20%) ⬇️
...rum-core/src/domain/contexts/foregroundContexts.ts 96.92% <0.00%> (-1.54%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ThibautGeriz ThibautGeriz force-pushed the thibaut.gery/fix-serialization-for-checkbox-radio branch from e01a66b to 21a9ba1 Compare February 17, 2023 13:37
@ThibautGeriz ThibautGeriz force-pushed the thibaut.gery/fix-serialization-for-checkbox-radio branch from 21a9ba1 to 157f74f Compare February 17, 2023 16:16
@ThibautGeriz ThibautGeriz marked this pull request as ready for review February 17, 2023 16:23
@ThibautGeriz ThibautGeriz requested review from a team as code owners February 17, 2023 16:23
)
})

it('serializes <input type="radio"> elements checked state with mask serialization', () => {
Copy link
Member

Choose a reason for hiding this comment

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

💬 suggestion:

  • this test title is a bit misleading as it actually does not serialize the checked state.
  • Maybe move it in the describe('input privacy mode mask-user-input' to colocate with similar tests

@@ -107,22 +107,29 @@ describe('serializeNodeWithId', () => {
isolatedDom.clear()
})

function serializeElement(
node: Node,
Copy link
Member

Choose a reason for hiding this comment

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

🥜 nitpick: ‏the node argument could have the type Element, since we are specifically serializing elements.

@ThibautGeriz ThibautGeriz merged commit 1eb0f6a into main Feb 22, 2023
@ThibautGeriz ThibautGeriz deleted the thibaut.gery/fix-serialization-for-checkbox-radio branch February 22, 2023 08:38
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.

3 participants