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

Get sources to set Tor Browser security slider to High #1480

Merged
merged 7 commits into from
Dec 6, 2016

Conversation

redshiftzero
Copy link
Contributor

SecureDrop’s source interface probably has one of the best use cases for the “High” security level of Tor Browser’s security slider. However, the icons we use looked bad/broken on the source interface using the “High” security level (described in #1024). This PR adds an explanation to the source guide telling them how to use the security slider, replaces all Font Awesome icons on the source interface with SVG graphics (as suggested by #1024 and #1477), and adds an explanation to the source interface if JavaScript is enabled to turn up the security slider to High. Note that turning up the security slider handles turning off JavaScript as well. I verified that nothing else breaks on the source interface other than the Font Awesome icons. Closes #1024.

@psivesely
Copy link
Contributor

Text on this page needs to be fixed. Otherwise, lgtm. 2016-12-02-113034_1517x890_scrot

Adding a task to #1476 to do the same for the JI.

@redshiftzero
Copy link
Contributor Author

Thanks for the review @fowlslegs! Updated and the page looks like this now:

screen shot 2016-12-02 at 6 25 06 pm

Copy link
Contributor

@psivesely psivesely left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now! Merging!

@psivesely psivesely merged commit 2fc4b01 into develop Dec 6, 2016
@psivesely psivesely deleted the security-slider branch December 6, 2016 01:19
@psivesely
Copy link
Contributor

psivesely commented Jan 11, 2017

We received an email:

Does anyone know when this merged PR will make it into a release?
#1480

Would it be possible for it to be kinda soon? :)

At [redacted] we're doing a series a videos on digital security that
[redacted] are hosting. One of them will be part of [redacted]'s
[redacted], and it will be a video tutorial [redacted]: [redacted]
including how to use SecureDrop as a source.

I'm writing the script for it and I've run into a usability issue. I
include visiting torproject.org in a normal browser, downloading Tor
Browser, opening it, and loading [redacted] SecureDrop. It has the big red bar
at the top that says, "We recommend disabling JavaScript to protect your
anonymity: Learn how to disable it, or ignore this warning to continue."
But the "Learn how to disable it" instructions are wrong (and only work
if you're security slider is already at low), and if I do change the
slider to high and reload all of the icons break. The PR fixes this.

So the choice right now, for the script of this video, is to either
ignore the warning, or to heed the warning but not the way it suggests
and end up with broken icons. Both of those options kind of suck.

I don't know exactly when we're filming, but probably in early February.
Would it be possible to get this PR in a release before then? If not, I
suppose worse case I could manually apply that PR to [redacted]'s instance.
But I'd rather avoid doing hacky things like that if I can help it.

If we are to make a 0.3.11 release, which seems probable and wise given our long list of TODOs for 0.4, I think we should try to include this PR. Thoughts @freedomofpress/securedrop-maintainers?

@psivesely psivesely added this to the 0.3.11 milestone Jan 17, 2017
@micahflee
Copy link
Contributor

I just tested this PR, and unfortunately doesn't completely fix the issue.

  • Unfortunately, it looks like (as of the latest Tor Browser) setting the security slider to High says, "Some types of images are disabled," and this appears to include svg images, which means that the images are still broken.
  • It doesn't update /etc/apparmor.d/usr.sbin/apache2 to allow apache2 to read all the new images in static/i/font-awesome

@micahflee
Copy link
Contributor

Also, it looks like some of the Tor Browser UI has changed, so the instructions text should be updated. It says to click the onion and choose "Privacy and Security Settings", but now it's just "Security Settings".

@micahflee
Copy link
Contributor

I also noticed a few other issues. Right now the buttons with icons in them are white on blue, and the css hover affect makes them blue on white. In order to retain this affect, each icon needs two versions of it, and the css needs to work in such a way that it switches to a different icon on hover.

Also, I noticed some other small style changes that seem to happen -- like, in /generate the button at the bottom has an icon and says "Continue", but when changing away from Font Awesome the icon is aligned differently, etc.

@heartsucker
Copy link
Contributor

@micahflee Could you screen shot this? I just made changes to these elements that made it into the mainline.

@micahflee
Copy link
Contributor

I took the svg files and converted them to 20x20 pngs. This is the index page (the button location, size, and icon vertical centering is all a bit off):

screenshot from 2017-02-06 21 10 08

And you can't see the mouse cursor in the image, but this is the index page when I'm hovering over "Submit Documents" -- the icon png is white on white, so you can't see it.

screenshot from 2017-02-06 21 10 43

Here's the "Continue" button on the generate page.

screenshot from 2017-02-06 21 11 06

This wasn't exhaustive at all, there could be some other issues too. But that was just what I quickly noticed. Everything should be checked thoroughly obviously.

@heartsucker
Copy link
Contributor

@micahflee Ah yes, one of @ninavizz's recommendation was actually to strip the icons from the buttons entirely, so those won't be there in the future (and aren't in current develop).

However, if SVGs don't work in TB with high security settings, then we might have some other problems as a few more have been added.

And as for the centering, it doesn't look like that in FF as of commit b83f219 (and I can't get TB to talk to localhost, so I can't confirm there).

@psivesely
Copy link
Contributor

@heartsucker Once you figure out the legitimate issues and branches affected it would be awesome if you could file them in the tracker. I think you're more on top of the UI/UX changes project than me, so you're in a better position to do this. Thanks!

@ninavizz
Copy link
Member

ninavizz commented Feb 6, 2017

Damn. I've been in touch with the TOR peeps on making that control more usable. :/ Will see what's going on with it. The slider should be a toolbar-button actionable control, the whole TBB setup as-is, stinks. I also had a minor meltdown when first new to Tor, trying to figure-out where that damn thing was.

@ninavizz
Copy link
Member

ninavizz commented Feb 9, 2017

Sorry—this is my 2nd late-to-the-game request, but could we possibly just change the language of the text on this page, to the attached? The text as-is, is quite overwhelming. Especially the red text (which also, being red, looks like an inline error message—even tho the user hasn't done anything). If this request sends eyes rolling or knuckles whitening, ignore! Looking forward to getting UX stuff more synced w/ release schedules (it ain't supposed to be done as separate projects, cough).

sd_generate

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

Successfully merging this pull request may close these issues.

5 participants