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

bugfix: Removve redundant content so that the textarea content in the replay will no longer appear repeatedly. #1326

Closed
wants to merge 1 commit into from

Conversation

YanJie-001
Copy link

Removve redundant content so that the textarea content in the replay will no longer appear repeatedly.
To fix [Bug]: Textarea shows default value twice in replay #1301

@changeset-bot
Copy link

changeset-bot bot commented Oct 13, 2023

⚠️ No Changeset found

Latest commit: 724d805

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@eoghanmurray
Copy link
Contributor

See my comments in #1301

@eoghanmurray
Copy link
Contributor

Ignoring the current failing tests which are related to _cssText which is presumably unrelated, this PR really needs a few exemplifying test cases to ensure there isn't a future regression on this (it looks like there was a few prior rounds of changes around this issue in the git history). Would you be able to supply that or would you prefer to wait until I can figure it out?

I imagine we'd want to add a <textarea> one of the test cases in packages/rrweb/test/record.test.ts in order to show that the repeated value is a problem.

@eoghanmurray
Copy link
Contributor

eoghanmurray commented Nov 9, 2023

As currently implemented, this PR modifies the tests as follows:

rrweb-snapshot:test:     -         <textarea name="" id="" cols="30" rows="10">1234</textarea>
rrweb-snapshot:test:     +         <textarea name="" id="" cols="30" rows="10" value="1234"></textarea>

This is less correct in that if the latter is rendered in HTML, then the value attribute is ignored (the .value parameter is available in javascript)

@eoghanmurray
Copy link
Contributor

I'm gonna close this one as #1351 goes into much more detail than this one by fixing at recording time, but also fixing for existing recordings where the bad data is already in the event (like this fix does)

@eoghanmurray
Copy link
Contributor

Forgot to say: thank you for this PR, I wouldn't have made my one if I could not see your intended changes!

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.

2 participants