search

sidstamm / FirefoxCertificateManager

Rose-Hulman Senior Project with Mozilla
5 stars 4 forks source link

Remove references to unsafeWindow at request of AMO reviewers #28

Closed sidstamm closed 8 years ago

sidstamm commented 8 years ago

unsafeWindow is used in data/scripts/inject.js, and if possible should be removed. What's it used for?

https://blog.mozilla.org/addons/2014/04/10/changes-to-unsafewindow-for-the-add-on-sdk/

MasonSchneider commented 8 years ago

Just to let you know, we submitted a new review because we wanted to remove the non-functional button for View all certs. UnsafeWindow is there because we needed to allow the add-on to populate the cert info on the front-end. It looks like the fix detailed in your article should be fairly simple to implement.

sidstamm commented 8 years ago

Ok. In general, ignoring the reviewer's suggestions is not helpful towards getting approved. Fixing this will probably help with review times and such.

MasonSchneider commented 8 years ago

Yeah, in the initial feedback they made it sound as if it was more of an inconvenience rather than something we would be required to fix.

sidstamm commented 8 years ago

"You really should not" written in the review might be code for "fix this".

sidstamm commented 8 years ago

FWIW, in the review for 1.0.2, it said "please fix for next time"