Resolve "null is not a valid URL" errors caused by a URL parsing bug in Firefox #192
Conversation
| try { | ||
| const tab = await chrome.tabs.get(tabId); | ||
| badgeCache.settings.origin = new URL(tab.url).origin; | ||
| badgeCache.settings.origin = tab.url && new BrowserpassURL(tab.url).origin; |
There was a problem hiding this comment.
I don't like that we have to repeat this tab.url && XXX logic in many places, easy to miss... Maybe we should think of another approach? e.g. always initialize BrowserpassURL object, but make the origin field inside of it be null, or some special value, so that we don't have to check for null everywhere?
There was a problem hiding this comment.
Are you suggesting that BrowserpassURL should accept null as a valid argument? Because that's what your suggestion implies.
There was a problem hiding this comment.
Is it ever null? When I tested this, tab.url was "about:newtab" in Firefox (and "chrome://newtab" in chromium).
There was a problem hiding this comment.
Yes, because that's not the only place we obtain a URL from. In some places we parse the origin property of a previously-parsed URL, which may be null.
There was a problem hiding this comment.
Also, this specific invocation sometimes has null coming from tab.url. This showed up in Firefox debugging.
There was a problem hiding this comment.
let's rethink it, how about we make settings.origin be not a nullable string, but non-nullable BrowserpassURL object that we don't have to ever reparse? If that requires allowing BrowserpassURL to accept null, we should consider that too :)
2 participants
What
Fixes #191.
Why
Because Firefox is annoyingly broken, again, and somehow thinks providing
"null"instead ofnullis reasonable.