Add restriction on tab opening to the current active tab #13
Conversation
src/background/index.ts
Outdated
| ), | ||
| ); | ||
| if (activeTab !== tabId) { | ||
| logger.debug('Not openning a new tab due to requesting tab not being active'); |
There was a problem hiding this comment.
typo:
| logger.debug('Not openning a new tab due to requesting tab not being active'); | |
| logger.debug('Not opening a new tab due to requesting tab not being active'); |
Observation: this function returns undefined, but the the caller also checks for null. Do you purposefully differentiate between them?
const redirectPromise = w3HeaderValue
.then(async (value): Promise<void | chrome.webRequest.BlockingResponse> => {
if (value === null) { // <-- null check
delete TOKENS[details.url];
return;
}
if (isManifestV3(chrome)) {
// ...
} else {
if (value) { // <-- undefined check
TOKENS[details.url] = value;
}
}
return { redirectUrl: details.url };
})There was a problem hiding this comment.
I return undefined because that's the normal return path of similar check in this function. However, this does indeed not seem to be a normal validation.
I've pushed a new commit with the correct check !value, which would cover undefined and null (or even empty string, even though we don't expect this value).
The typo has been corrected as well.
There was a problem hiding this comment.
With the !value change, the second if (value) { check is redundant, it will always be true. LGTM otherwise!
thibmeu
force-pushed
the
feature-limit-tab-opening
branch
from
3139f4f
to
e1af955
Compare
thibmeu
force-pushed
the
feature-limit-tab-opening
branch
2 times, most recently
from
9ef65bf
to
69ab8b4
Compare
In the existing extension, any tab initiating a PrivateToken request for a trusted issuer is going to trigger the associated attester tab opening. This might interrupt the user flow. This commit introduces an improvement to only allow the current active tab to open a new tab upon a PrivateToken request.
`headerToToken` returns `Promise<string | undefined>`. However, the existing check assessed `headerToToken().then(value => value === null ? something : somethingElse)`. This should never be true. Instead, this check is replaced with `!value`.
thibmeu
force-pushed
the
feature-limit-tab-opening
branch
from
69ab8b4
to
c8be6ce
Compare
|
PR updated to move from |
In the existing extension, any tab initiating a PrivateToken request for a trusted issuer is going to trigger the associated attester tab opening. This might interrupt the user flow.
This commit introduces an improvement to only allow the current active tab to open a new tab upon a PrivateToken request.