Skip to content

Extended text masking function to include relevant HTMLElement#1310

Merged
eoghanmurray merged 13 commits intorrweb-io:masterfrom
PostHog:fix/text-masking-fn
Oct 13, 2023
Merged

Extended text masking function to include relevant HTMLElement#1310
eoghanmurray merged 13 commits intorrweb-io:masterfrom
PostHog:fix/text-masking-fn

Conversation

@benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented Sep 26, 2023 •
edited
Loading

A while back I added the element-to-be-masked to the masking function for inputs. This offers ultimate flexibility as the implementor can re-implement any of the other masking checks or any other thing they can come up with.

What was missing was the corresponding thing for text. This PR adds it.

For example it can solve the request in this issue without needing to add more, conflicting options.

{
    maskAllInputs: true,
    maskInputFn: (text, element) => {
        if (element?.dataset['rr-unmask'] === 'true') {
            return text
        }
        return '*'.repeat(text.length)
    },
    maskTextSelector: "*",
    maskTextFn: (text, element) => {
        if (element?.dataset['rr-unmask'] === 'true') {
            return text
        }
        return '*'.repeat(text.length)
    },
}

With this example, everything will be masked except elements with the data attribute rr-unmask.

@changeset-bot
Copy link

changeset-bot bot commented Sep 26, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 0c26420

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
rrweb-snapshot Minor
rrweb Minor
rrdom Minor
rrdom-nodejs Minor
rrweb-player Minor
@rrweb/types Minor
@rrweb/web-extension Minor
rrvideo Minor

Not sure what this means? Click here to learn what changesets are.

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

@benjackwhite benjackwhite mentioned this pull request Sep 26, 2023
Closed
@ArkadyFukzon
Copy link

ArkadyFukzon commented Sep 26, 2023

+1 for this, really useful change

Juice10
Juice10 reviewed Oct 6, 2023
View reviewed changes
Juice10
Juice10 reviewed Oct 6, 2023
View reviewed changes
benjackwhite and others added 2 commits October 6, 2023 12:00
@benjackwhite @Juice10
Update packages/rrweb/src/record/mutation.ts
e38e6e7 
Co-authored-by: Justin Halsall <Juice10@users.noreply.github.com>
@benjackwhite @Juice10
Update packages/rrweb/src/record/mutation.ts
e997fc1 
Co-authored-by: Justin Halsall <Juice10@users.noreply.github.com>
@benjackwhite benjackwhite requested a review from Juice10 October 6, 2023 10:41
@benjackwhite
Copy link
Contributor Author

benjackwhite commented Oct 9, 2023

@Juice10 made the changes 👍

Juice10
Juice10 approved these changes Oct 10, 2023
View reviewed changes
Copy link
Member

@Juice10 Juice10 left a comment

Choose a reason for hiding this comment

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

Great addition @benjackwhite thanks!

eoghanmurray
eoghanmurray approved these changes Oct 13, 2023
View reviewed changes
Copy link
Contributor

@eoghanmurray eoghanmurray left a comment

Choose a reason for hiding this comment

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

naming suggestion

getElementFromNode sounded like a built in getElementById and hid the 'find parent' aspect.

the "closest" idea has precedence in https://developer.mozilla.org/en-US/docs/Web/API/Element/closest but maybe I'm overthinking it?

benjackwhite and others added 5 commits October 13, 2023 12:35
@benjackwhite @eoghanmurray
Update packages/rrweb/src/record/mutation.ts
bd8fb92 
Co-authored-by: Eoghan Murray <eoghan@getthere.ie>
@benjackwhite @eoghanmurray
Update packages/rrweb/src/record/mutation.ts
4d540ef 
Co-authored-by: Eoghan Murray <eoghan@getthere.ie>
@benjackwhite
Update packages/rrweb/src/utils.ts
60f42d4
@benjackwhite @eoghanmurray
Update packages/rrweb/src/utils.ts
f31142a 
Co-authored-by: Eoghan Murray <eoghan@getthere.ie>
@benjackwhite
Fix
0c26420
@benjackwhite
Copy link
Contributor Author

benjackwhite commented Oct 13, 2023

@eoghanmurray committed your changes (an then corrected the typos that I didn't initially notice 😅 )
Naming definitely makes sense

@eoghanmurray eoghanmurray mentioned this pull request Oct 13, 2023
Open
@eoghanmurray eoghanmurray merged commit 7c0dc9d into rrweb-io:master Oct 13, 2023
billyvg pushed a commit to getsentry/rrweb that referenced this pull request Oct 20, 2023
@benjackwhite @Juice10
@eoghanmurray @billyvg
Extended text masking function to include relevant HTMLElement (rrweb…
50dde79 
…-io#1310)

* Extends maskTextFn to pass the HTMLElement to the deciding function

---------

Authored-by: benjackwhite <benjackwhite@users.noreply.github.com>
Co-authored-by: Justin Halsall <Juice10@users.noreply.github.com>
Co-authored-by: Eoghan Murray <eoghan@getthere.ie>
@daibhin daibhin deleted the fix/text-masking-fn branch November 1, 2023 11:01
This was referenced Dec 4, 2023
Merged
Open
Closed
billyvg pushed a commit to getsentry/rrweb that referenced this pull request Jan 31, 2024
@benjackwhite @Juice10
@eoghanmurray @billyvg
Extended text masking function to include relevant HTMLElement (rrweb…
ed1a2ed 
…-io#1310)

* Extends maskTextFn to pass the HTMLElement to the deciding function

---------

Authored-by: benjackwhite <benjackwhite@users.noreply.github.com>
Co-authored-by: Justin Halsall <Juice10@users.noreply.github.com>
Co-authored-by: Eoghan Murray <eoghan@getthere.ie>
@billyvg billyvg mentioned this pull request Jan 31, 2024
Merged
@github-actions github-actions bot mentioned this pull request Feb 28, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Juice10 Juice10 Juice10 approved these changes

@eoghanmurray eoghanmurray eoghanmurray approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@benjackwhite @ArkadyFukzon @Juice10 @eoghanmurray