Skip to content
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

Feedback from Mozilla reviewer #70

Open
thieman opened this issue Jan 11, 2016 · 4 comments
Open

Feedback from Mozilla reviewer #70

thieman opened this issue Jan 11, 2016 · 4 comments
Assignees

Comments

@thieman
Copy link
Owner

thieman commented Jan 11, 2016

  1. Remove data/background.js. This file is only relevant for a Chrome extension.

  2. data/selfie.js: When using the message event, always verify that the source is okay, otherwise you may accidentally expose your add-ons functionality to arbitrary pages.
    See https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage
    For same-page communications, I recommend CustomEvent over postMessage, because the above issue will not occur, and your message event will not conflict with other (badly written) add-ons that do not expect your message events.
    See https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Creating_and_triggering_events and
    https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent

  3. Overwriting pushState / replaceState and modifying the CSP is nasty. Try to avoid it if possible. There are several alternatives, e.g. usinga MutationObserver to watch an element for changes. If you really want to overwrite history.pushState, then use exportFunction, see https://developer.mozilla.org/en-US/Add-ons/SDK/Guides/Content_Scripts/Interacting_with_page_scripts#Expose_functions_to_page_scripts and https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.exportFunction

@bhollis
Copy link
Collaborator

bhollis commented Jan 11, 2016

OK. I'll have to see whether I can force myself to care enough about Firefox to fix those...

@bhollis
Copy link
Collaborator

bhollis commented Jan 11, 2016

Part of the problem is me wanting to have the same code on Chrome and FF.

@thieman
Copy link
Owner Author

thieman commented Jan 11, 2016

OK. I'll have to see whether I can force myself to care enough about Firefox to fix those...

@bhollis bhollis self-assigned this Jan 13, 2016
@bhollis
Copy link
Collaborator

bhollis commented Jan 31, 2016

I removed background.js. Haven't done the rest yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants