Fix the issue where reading the cookies from a different domains#16
Fix the issue where reading the cookies from a different domains#16
Conversation
erik-beus
left a comment
erik-beus
left a comment
There was a problem hiding this comment.
Please see suggestions/questions
chrome/src/background.ts
Outdated
| chrome.webNavigation | ||
| .getFrame({ | ||
| documentId: info.parentDocumentId, | ||
| frameId: info.parentFrameId, | ||
| }) | ||
| .then((parentFrame) => { | ||
| if (!parentFrame) { | ||
| return undefined | ||
| } | ||
|
|
||
| const parentUrl = new URL(parentFrame.url) | ||
| if ( | ||
| parentUrl.host.endsWith('toddle.dev') === false && | ||
| parentUrl.host.endsWith('nordcraft.com') === false | ||
| ) { | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
This logic seems to be a duplicate of the code on line 17->36? Consider moving this to a separate function to make it easier to maintain - especially when (soon) need to remove 'toddle.dev' as a valid domain for the parent frame.
chrome/src/background.ts
Outdated
| } | ||
| }, | ||
| removeCookie: async (cookie, domain) => { | ||
| await chrome.cookies.remove(cookie) |
There was a problem hiding this comment.
Does this one also take a domain as argument?
chrome/src/background.ts
Outdated
| if (info.parentFrameId < 0) { | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
| if (info.parentFrameId < 0) { | |
| return | |
| } |
| } | ||
|
|
||
| // check the parent frame so we only override cookies if we are on nordcraft.com | ||
| nordcraftIsParentFrame({ |
There was a problem hiding this comment.
Is it possible to make the event listener asynchronous on line 65 so you can use await here?
There was a problem hiding this comment.
Nope, I wasn't able to do it async so that's why I did it like this.
chrome/src/background.ts
Outdated
| await chrome.cookies.set(cookie) | ||
|
|
||
| const parsedUrl = new URL(cookie.url) | ||
|
|
| }) | ||
| } | ||
|
|
||
| export async function nordcraftIsParentFrame({ |
There was a problem hiding this comment.
Could you do something similar for Firefox as well?
2 participants
No description provided.