report(dom): handle undefined link url from proto roundtrip #12872
Conversation
| */ | ||
| safelySetHref(elem, url) { | ||
| // Defaults to '' to fix proto roundtrip issue. | ||
| url = url || ''; |
There was a problem hiding this comment.
doing this instead of default param value, because that makes typescript make the second param optional.
patrickhulce
left a comment
patrickhulce
left a comment
There was a problem hiding this comment.
thanks @derevandal !
| * Set link href, but safely, preventing `javascript:` protocol, etc. | ||
| * @see https://github.com/google/safevalues/ | ||
| * @param {HTMLAnchorElement} elem | ||
| * @param {string} url |
There was a problem hiding this comment.
| * @param {string} url | |
| * @param {string|undefined} url |
There was a problem hiding this comment.
I'd prefer not littering our types with this, just because a use case comes to us from proto. If we were to go down that route, we should turn all string into string|undefined in the report.
We've got to address the root cause eventually, maybe changes like this would be included in that.
There was a problem hiding this comment.
we should turn all string into string|undefined in the report.
I agree :)
If you want to postpone this for a more thorough examination of all usage that wfm though.
Co-authored-by: Patrick Hulce <[email protected]>
4 participants
ref #12868
Commit with message: