Enable scrubbing options for dom inputs#1383
Conversation
matux
left a comment
matux
left a comment
There was a problem hiding this comment.
This looks great! I think I found something in the scrubInputValue logic where I'm not sure what it does is what we want, so left two comments.
| this.maskInputOptions[inputType] | ||
| ) { | ||
| if (this.maskInputFn) { | ||
| value = this.maskInputFn(value, element); |
There was a problem hiding this comment.
If value happens to be masked in the conditionals in line 802:813, you'd be passing the masked value to this.maskInputFn here.
Maybe store the value in some originalValue var and pass that here?
| if ( | ||
| domUtil.isMatchingElement(element, this.scrubClasses, this.scrubSelectors) | ||
| ) { | ||
| value = mask; | ||
| } | ||
|
|
||
| if (inputType === 'password') { | ||
| value = mask; | ||
| } |
There was a problem hiding this comment.
Could this override the return value of the custom this.maskInputFn? Is that expected behavior?
There was a problem hiding this comment.
Yes, this is expected. If it's a password field, it is always masked. Making it an early return at the top would make that more obvious.
| ((options = { scrubFields, autoInstrument: { log: false } }), | ||
| (rollbar = new Rollbar({}))); |
There was a problem hiding this comment.
non-blocking, don't feel strongly, just asking
Is there any reason this is like this and not just
options = { scrubFields, autoInstrument: { log: false } };
rollbar = new Rollbar({});?
There was a problem hiding this comment.
I think prettier did this? Let me see what happens when I undo it.
| @@ -8,7 +8,7 @@ | |||
| <div class="container" id"main-container"> | |||
There was a problem hiding this comment.
Yikes, there's an = missing between id and "main-container" here!
2 participants
Description of the change
Enables scrubbing of DOM inputs in telemetry.
telemetryScrubberandscrubTelemetryInputsas documented here: https://docs.rollbar.com/docs/rollbarjs-telemetryType of change
Checklists
Development