Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

various issues with noscript icon #8403

Closed
diracdeltas opened this issue Apr 19, 2017 · 11 comments · Fixed by #8410
Closed

various issues with noscript icon #8403

diracdeltas opened this issue Apr 19, 2017 · 11 comments · Fixed by #8410

Comments

@diracdeltas
Copy link
Member

diracdeltas commented Apr 19, 2017

Test plan

#8410 (comment)


  1. disable scripts on a page then click the noscript icon, there is a console error:
"Refused to send form data to 'chrome://brave/Users/yan/repos/browser-laptop/app/extensions/brave/index-dev.html#' because it violates the following Content Security Policy directive: "form-action http://localhost:*".
  1. click the noscript icon repeatedly and the popup doesn't show up after the first time.

  2. when noscript is enabled, twitter.com is missing the lock icon:
    screen shot 2017-04-19 at 22 03 29

  3. the clickable area should be increased

@diracdeltas
Copy link
Member Author

diracdeltas commented Apr 19, 2017

cc @bsclifton. this should block release if it's in the built version of 0.15.0 and other people also have this bug.

@diracdeltas diracdeltas added this to the 0.15.0 milestone Apr 19, 2017
@diracdeltas diracdeltas changed the title clicking on noscript icon throws CSP error various issues with noscript icon Apr 19, 2017
@diracdeltas diracdeltas self-assigned this Apr 19, 2017
@bsclifton
Copy link
Member

bsclifton commented Apr 19, 2017

The popup works as expected for me using 0.14.2 RC2, however the browser dev tools do log a CSP error:
screen shot 2017-04-19 at 3 16 43 pm

  • lock icon disappears after performing steps on HTTPS site
  • unlocked icon stays visible after performing steps on HTTP site

@diracdeltas
Copy link
Member Author

if i click on the noscript icon 3 times in a row, it stops toggling the popup after the 2nd click

@bsclifton
Copy link
Member

@diracdeltas I can't repro that in dev-channel:
noscript

diracdeltas added a commit that referenced this issue Apr 19, 2017
fix Part 1 of #8403

test plan:
1. disable scripts on a page
2. click noscript icon
3. open browser console. you should not see any CSP errors.
@diracdeltas
Copy link
Member Author

@bsclifton pretty sure that issue was due to redux refactoring and i have a fix

@diracdeltas
Copy link
Member Author

is anyone seeing the lock icon issue on a site other than twitter.com? i think it's due to the noscript twitter siteHack actually

@bsclifton
Copy link
Member

bsclifton commented Apr 20, 2017

@diracdeltas I tried a few sites, can't repro the lock disappearing-
facebook.com, google.com, wikipedia.org

@cezaraugusto
Copy link
Contributor

cezaraugusto commented Apr 20, 2017

reproduced here, SHA f4d52d1

", source: chrome://brave/Users/cezaraugusto/dev/browser-laptop/app/extensions/brave/index-dev.html (0)
[29361:775:0420/042259.013490:ERROR:CONSOLE(0)] "Refused to send form data to 'chrome://brave/Users/cezaraugusto/dev/browser-laptop/app/extensions/brave/index-dev.html#' because it violates the following Content Security Policy directive: "form-action http://localhost:*".

@cezaraugusto
Copy link
Contributor

not only twitter but google as well

@bsclifton
Copy link
Member

@cezaraugusto you can reproduce the CSP error, but does the lock icon disappear?

@cezaraugusto
Copy link
Contributor

yep, but for twitter only:

screen shot 2017-04-20 at 4 40 20 am

diracdeltas added a commit that referenced this issue Apr 20, 2017
fix #8403

1. change noscript icon from button to span to fix CSP error
2. fix repeated toggling of noScriptInfo dialog
3. refactor to remove deprecated code and combine windowActions
4. increase noscript click area
5. fix security icon disappearing on twitter.com

test plan:
1. disable scripts on twitter.com. the lock icon should not disappear
2. click noscript icon
3. open browser console. you should not see any CSP errors.
4. now click the noscript icon repeatedly. you should see the popup come up and disappear repeatedly.
bsclifton pushed a commit that referenced this issue Apr 20, 2017
fix #8403

1. change noscript icon from button to span to fix CSP error
2. fix repeated toggling of noScriptInfo dialog
3. refactor to remove deprecated code and combine windowActions
4. increase noscript click area
5. fix security icon disappearing on twitter.com

test plan:
1. disable scripts on twitter.com. the lock icon should not disappear
2. click noscript icon
3. open browser console. you should not see any CSP errors.
4. now click the noscript icon repeatedly. you should see the popup come up and disappear repeatedly.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.