-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scope approvals to documentId #49
Conversation
🦋 Changeset detectedLatest commit: 1133bcb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
d03889a
to
d706680
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
elaborated some of the comments and parameter naming in various places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no logic or behavior change in this code, but its behavior is clearer now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no longer using chrome.tabs.onUpdated, just listening for content scripts to announce themselves
} else { | ||
respond(PraxConnection.Denied); | ||
// something strange is happening. either storage is broken, the popup | ||
// returned an error, the sender is invalid, or someone's misbehaving. | ||
// obfuscate this rejection with a random delay 2-12 secs | ||
setTimeout(() => respond(PenumbraRequestFailure.Denied), 2000 + Math.random() * 10000); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addition
void chrome.tabs.sendMessage(validSender.tab.id, PraxConnection.Init, { | ||
documentId: validSender.documentId, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requests now also use chrome.tabs.sendMessage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validation extracted to a fairly comprehensive fn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validation tests
423b888
to
cccba15
Compare
console.warn('Connection request listener failed:', e); | ||
} | ||
// something is wrong. user may not have seen a popup. | ||
if (globalThis.__DEV__) console.warn('Connection request listener failed:', e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
brackets plz
cf610c5
to
1133bcb
Compare
replaces #42
validation is called in the handler before attempting to identify approval