Skip to content

fix: Ensure <input type='submit' value='Btn text'> is masked#69

Merged
mydea merged 2 commits intosentry-v1from
fn/mask-button-value
Mar 1, 2023
Merged

fix: Ensure <input type='submit' value='Btn text'> is masked#69
mydea merged 2 commits intosentry-v1from
fn/mask-button-value

Conversation

@mydea
Copy link
Copy Markdown
Member

@mydea mydea commented Feb 28, 2023

This should be considered "Text", and should thus be masked by maskAllText.

This is both for <input type='button'> as well as <input type='submit'>.

Closes #68

This should be considered "Text", and should thus be masked by `maskAllText`.
@mydea mydea requested review from Lms24 and billyvg February 28, 2023 09:55
@mydea mydea self-assigned this Feb 28, 2023
@mydea mydea changed the title fix: Ensure <input type='button' value='Btn text'> is masked fix: Ensure <input type='submit' value='Btn text'> is masked Feb 28, 2023
Copy link
Copy Markdown
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

Comment on lines +549 to +550
element,
element.tagName,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

l: We probably don't need both of these parameters

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah, I thought the same, wasn't sure - in one call site there is this getValidTagName method, not sure if we need this here...?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can always revisit this/clean this up later.

@mydea mydea merged commit 5df2651 into sentry-v1 Mar 1, 2023
@mydea mydea deleted the fn/mask-button-value branch March 1, 2023 08:25
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.

maskAllText should mask button value

3 participants