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

ToDo: compare active values ghacks vs pyllyukko user.js #18

Closed
15 tasks done
Thorin-Oakenpants opened this issue Feb 21, 2017 · 8 comments
Closed
15 tasks done

ToDo: compare active values ghacks vs pyllyukko user.js #18

Thorin-Oakenpants opened this issue Feb 21, 2017 · 8 comments
Assignees

Comments

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Feb 21, 2017

After #10 ghacks mini diff and #208 pyllyukko monster diff ToDo's ae finished, do a diff of active values and investigate

DONE

  • pref("network.stricttransportsecurity.preloadlist", true); '' ghacks: false and commented out
  • pref("network.cookie.cookieBehavior", 1); // ghacks: 2
    • no change. we choose to disallow all and tell users to use an extension for granular control
  • pref("pdfjs.disabled", true); // ghacks: false
    • no change. I would rather by default use the inbuilt reader. The info is all there in the pref number description, and informed users can change to external and use anything that they consider more secure/lightweight. We don't know what every end user has as a pdf app on their system - forcing an end user by default to this is a bad strategy for us. One word .. Acrobat.
  • pref("privacy.clearOnShutdown.cookies", true); // ghacks: false
    • no change: We do not want to destroy peoples cookies, especially when our default is block all and use an extension to control them. The only other thing we don't clear on shutdown is site preferences (which would upset a lot of users if sprung on them). The info is there if someone wants to clear absolutely everything
  • pref("privacy.clearOnShutdown.sessions", true); // ghacks: false
    • no change: see above
  • pref("extensions.update.enabled", true); // ghacks: false
    • no change: we think manual updates allow flexibility for reasons stated in the pref descriptions. Note for this and SB + TP, the readme and descriptions and js section header will point these particular pitfalls out (there will be iittle else in the section header, so it will stand out) - that's THREE places the user has been told in hard to miss terms, and the onus is on them
  • pref("browser.safebrowsing.malware.enabled", true); // ghacks: false
    • no change: we choose to -SB&TP and +uBo etc
  • pref("privacy.trackingprotection.enabled", true); // ghacks: false
    • no change: same as SB above
  • pref("privacy.trackingprotection.pbmode.enabled", true); // ghacks: false
    • no change: same as SB above
  • pref("browser.aboutHomeSnippets.updateUrl", ""); // ghacks: "https://127.0.0.1"
    • no change: pyllyukko should match .. use HTTPS re MiTM re as per TBB and discussions there over this in tor tickets
  • pref("browser.newtabpage.directory.source", "data:text/plain,{}"); // ghacks: "data:text/plain,"
    • no change: keep same as TBB, should never be used due to master switches
  • pref("media.gmp-manager.url", ""); // ghacks: "data:text/plain,"
    • no change: see above
  • pref("browser.newtabpage.directory.ping", ""); // ghacks: "data:text/plain,"
    • no change: see above
  • pref("security.ask_for_password", 0); // ghacks: 2
    • no change: it must be 2 for security.password_lifetime to apply
  • pref("signon.storeWhenAutocompleteOff", false); // ghacks true
    • see discussion here pyllyukko/user.js@5e2e577
    • no change: pros and cons. Pros: possibly encourages more complex password use, is convenient, and why should sites dictate when I can and can't remember passwords. Cons: some sites should never remember passwords, like bank sites. At least with this setting, users can make their own mind up. I'll add something to the description
@earthlng
Copy link
Contributor

earthlng commented Feb 21, 2017

Oh, damn, so only when it's finished huh? Well I saw that too late so I adjusted my script, grabbed both versions latest master and compared them. The result is ... well, a lot of work still to be done....

combined prefs:  520
- 114 matching prefs
- 406 diffs
>   79 missing in ghacks
>  314 missing in pyllyukko
>   13 different values

The 314 are including the 23 parrot prefs.
I can provide a diff whenever one is needed, takes literally seconds to create them.

@earthlng
Copy link
Contributor

I didn't look at the file-content tbh, my script parsed all active prefs, idk if deprecated are in there for pyllyukko's. here is the output: http://pasted.co/b0235e9e

@earthlng
Copy link
Contributor

earthlng commented Feb 21, 2017

I see now, your script doesn't discard the status of the pref
No, of course not - you said do a diff of active values and investigate
I can adjust my script if you like, but I need to figure out how to best display that.

@earthlng
Copy link
Contributor

earthlng commented Feb 21, 2017

pref("browser.newtabpage.directory.source", "data:text/plain,{}"); // ghacks: "data:text/plain,"
@Thorin-Oakenpants, @pyllyukko, we should both use "data:,{}" - the code expects a JSON and complains otherwise, and with that format it defaults to "text/plain" and it also sets a charset that way, which again otherwise it can cause a warning in the console.
see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs
ie. data:, is the short form for data:text/plain;charset=US-ASCII,

@earthlng
Copy link
Contributor

earthlng commented Feb 21, 2017

creates an empty directoryLinks.json file in the appdata/local Mozilla folder
it creates that file with {} in it, but in the profile folder in my case. ie it's not an empty file and the folder is wrong afaik. everything after the comma is the file content. data:text/plain, would write an empty file, but it doesn't because it's not JSON while data:text/plain,{} creates a valid "empty" 2-byte JSON file.
I think this behavior is preferable because it retro-actively overwrites the content of that file, while otherwise, the code fails, produces a warning in the console and doesn't write/overwrite that file at all, if I remember correctly.

We have 5 data:text/plain, and 0 data:text/plain,{} atm!

@Atavic
Copy link

Atavic commented Feb 21, 2017

  • data:,
  • data:text/plain,
  • data:text/plain;charset=US-ASCII,

@earthlng
Copy link
Contributor

earthlng commented Feb 27, 2017

I don't understand why we should use the worst of the 3 possible ways. (see Atavic's comment)
Either use the shortest which sets a default (that I trust mozilla is not gonna change), or make it bullet-proof and use the long version.
Yeah, I read the TBB ticket where they started using it, but they also don't want any console warnings/errors, and tbh it seemed kind of lazy. We shouldn't do something stupid just because TBB also does it. Maybe someone from their team will eventually see our better and improved way and change it in TBB too.
Just my 2 cents

@earthlng
Copy link
Contributor

But they never get used

Then why change them at all? But we do change them (and I think we should) so why not use the best possible way instead of the worst?
I'm not gonna argue anymore. I use the short form in my own user.js. Do what you want.

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

No branches or pull requests

3 participants