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
Assignees
@diracdeltas
Labels
QA/checked-Linux QA/checked-macOS QA/checked-Win32 QA/checked-Win64 QA/test-plan-specified regression release-notes/exclude
Milestone
0.15.0

Comments

@diracdeltas
Copy link
Member

diracdeltas commented Apr 19, 2017 •
edited by luixxiul
Loading

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 •
edited
Loading

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 •
edited
Loading

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
@diracdeltas
change noscript icon from button to span
82e71d0 
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 •
edited
Loading

@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 •
edited
Loading

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
@diracdeltas
fix various noscript issues
7990e04 
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.
@diracdeltas diracdeltas mentioned this issue Apr 20, 2017
Merged
4 tasks
bsclifton pushed a commit that referenced this issue Apr 20, 2017
@diracdeltas @bsclifton
fix various noscript issues
3f3e167 
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.
This was referenced Apr 21, 2017
Closed
Closed
Closed
This was referenced Mar 18, 2018
Closed
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@diracdeltas diracdeltas

Labels
QA/checked-Linux QA/checked-macOS QA/checked-Win32 QA/checked-Win64 QA/test-plan-specified regression release-notes/exclude
Projects
None yet
Milestone
0.15.0
Development

Successfully merging a pull request may close this issue.

fix various noscript issues brave/browser-laptop
6 participants
@diracdeltas @luixxiul @cezaraugusto @bsclifton @NejcZdovc @srirambv