♻️ [PANA-5105] Serialize all DOM attribute values as strings #3999
+21
−44
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The current DOM serialization algorithm encodes two specific DOM attributes as boolean values:
checkedattribute found in<input type="checkbox">and<input type="radio">.selectedattribute found in<option selected>.I'd like to avoid this special case handling in the new DOM serialization algorithm. Instead, I'd like to encode these attributes as strings, just like all other attributes. Since these are boolean attributes, the standard string encoding for the value is:
{ selected: '' }(the empty string) for true. (e.g.<option selected>){}(no value recorded) for false. (e.g.<option>)This is exactly the transformation that the session replay player already applies to boolean-valued attributes, so the result will be the same at playback time. Indeed, handling things this way is less work on both ends! The existing player will just work with this approach.
I'd like to take this opportunity to very slightly change the behavior of
<option selected>, resolving an issue that was discovered when building the test suite added in #3994. Currently, there's a very particular circumstance where we do the wrong thing. The problematic situation is when an<option>element is masked, and:selectedDOM attribute.selectedproperty is false. (Because the user has selected another<option>since page load, perhaps.)In this situation, we will add the
selectedDOM attribute to the element's list of attributes. Then, we will see that theselectedproperty is false. We won't update the element's list of attributes, intending to omit theselectedproperty, but because theselectedproperty from the DOM is already there, we'll end up serializing it and including it in the element's attribute list in the recording. This is wrong both from a privacy perspective (we shouldn't capture this attribute on masked elements) and because it could cause us to show the wrong<option>as selected at replay time.The fix is always delete
selectedfrom the element's list of attributes if theselectedproperty is false. This aligns with what we do forchecked, so in addition to fixing this bug, it ends up simplifying the tests and reducing the number of different attribute serialization behaviors we need to reason about.Changes
Following the plan above, this PR:
checkedandselected. Now, the attribute value is the empty string when these attributes are logically true, and the attributes are not recorded when they are logically false.selectedbug described above.serializeDOMAttributes()and related functions, since they can no longer produce boolean values.Test instructions
It's enough to visit a page with an HTML
<select>dropdown, a set of checkbox inputs, or a set of radio button inputs, and look at the generated session replay using the browser SDK extension. We should continue to show the correct checked or selected item after the change.Checklist