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

README: refactor addons section; other fixes; fix GEOLOCATION/QUIET FOX sections of #208 #255

Merged
merged 20 commits into from
Apr 3, 2017
Merged

Conversation

nodiscc
Copy link
Contributor

@nodiscc nodiscc commented Mar 25, 2017

See commit messages

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 25, 2017

Done with this part. Let me know if this should be split into separate PRs.

@nodiscc nodiscc changed the title README: refactor addons section; other fixes README: refactor addons section; other fixes; fix GEOLOCATION/QUIET FOX sections of #208 Mar 26, 2017
link to mozilla meta tracking page for this pref
change pref description since this is related to more than screen size and media queries
 * remove lengthy description, just recommend addons, or don't recommend them.
 * remove duckduckgo plus, add generic hardening recommendation about privacy-conscious search engines
 * store all addon-specific recommendations in the addon section
 * Move ghostery to Other addons subsection, add uMatrix to this section
 * remove links to issues (use the search feature)
* move built-in tracking protection links to user.js tracking protection section
* run make - auto-update 'What does it do' section
…3 0204 of #208

more accurate description of accept_language setting
see https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#1350
set experiments manifest uri to blank, even though experiments are already disabled;
update previously broken links in experiments/telemetry section
fixes item 0340 of #208
… don't nag user about unsent crash reports: fixes items 0350 0351 of #208
README:
 * add fingerprinting mitigation to project goals
 * add optional user.js customization to usage steps
 * reorder system wide installation section (os specific steps at the end)
 * add generic hardening recommendations regarding password policy/open-source software/plain-text communications/tor usage
 * issues: add generic organization guideline
 * add firefox about: urls doc link

user.js:
 * add notice about webrtc breaking p2p file transfer methods
 * make affected Firefox versions consistent
 * detect preferences that are no longer present in firefox source
     target firefox version is configurable
     mozilla test suite prefs files are included for completeness sake
 * detect Firefox preferences not covered by user.js
     preferences that should we don't want to modify should be listed in ignore.list
     add a basic ignore.list
 * display stats on number of covered preferences
 * to use, run 'make tests'
     subtargets: downloadffprefs, checkdeprecated, stats, cleanup
 * add 'make tests' to travis script
 * run 'make authors' to generate an AUTHORS file, ordered by number of commits
    to add extra authors/credits, git commit --allow-empty --author="A U Thor <author@example.com>"
    TODO: add a .mailmap file to deduplicate authors with multiple email addresses

Results of tests against lastest Firefox revision can be seen at: ................
@Thorin-Oakenpants
Copy link

@nodiscc : I see under this you listed a link for about: URLs

I find using about:about will display everything applicable to your profile.

Compared to the list at the URL, I am missing:

  • about:app-manager - deprecated
  • about:compartments - deprecated
  • about:customizing - deprecated
  • about:permissions - deprecated (although the page notes it was removed in FF45)
  • about:sync-progress - valid entry, I have never set up sync
  • about:reader - valid entry, I just have reader disabled

And here are six items (non-extension) not listed

  • about:checkerboard
  • about:debugging
  • about:devtools-toolbox
  • about:performance
  • about:profiles
  • about:serviceworkers

Might be better and easier to just use about:about in the readme, as it will always be current and correct regardless of version

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 28, 2017

Thanks, I edited https://developer.mozilla.org/en-US/Firefox/The_about_protocol and asked for review. It's better to link to that page since it's the actual documentation on all about: pages. about:about: is listed there. I added a note that available about: pages may vary depending on Firefox versions.

Edit: page history https://developer.mozilla.org/en-US/Firefox/The_about_protocol$history

@nodiscc
Copy link
Contributor Author

nodiscc commented Mar 28, 2017

e92362c 093bd97 c0962da 24a5cfe c37bf58

README: update reference links, usage/installation/goals/hardening/FAQ doc (c0962da)
user.js: add a notice, cleanup
Makefile: add tests to detect deprecated/not covered prefs

Results of make tests introduced in c37bf58 can be seen at https://gist.github.com/nodiscc/ed0c84d3492e7faf8869ed52b5647dbd

Edit: also on travis: https://travis-ci.org/pyllyukko/user.js/builds/215770898

Mehmet Atif Ergun <mehmetaergun@users.noreply.github.com>
mengele-chan <mengele-chan@yandex.ru>
uberspot <onexemailx@gmail.com>
zummuz <zummuz@users.noreply.github.com>
Copy link
Owner

Choose a reason for hiding this comment

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

This is not needed. All the info is available as metadata.


// PREF: Disable Firefox Sync
// https://wiki.mozilla.org/Services/Sync
user_pref("services.sync.enabled", false);
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this is necessary, as it's opt-in and some users might actually want to use it.

@pyllyukko
Copy link
Owner

Will continue the review later, as I'm afraid I don't have the time to do it this week ☹️


// PREF: Don't use Mozilla-provided location-specific search engines
user_pref("browser.search.geoSpecificDefaults", false);
user_pref("browser.search.geoSpecificDefaults.url", "");
Copy link
Owner

Choose a reason for hiding this comment

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

Is the "" necessary, when the feature is toggled off?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not, this can be moved to ignore.list

Copy link
Owner

Choose a reason for hiding this comment

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

@./gen-readme.sh

# To decrease tests verbosity, comment out unneeded targets
tests: downloadffprefs checknotcovered checkdeprecated stats cleanup
Copy link
Owner

Choose a reason for hiding this comment

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

This will fail horribly if you have e.g. MAKEFLAGS="-j 4", like I do.

I'm not entirely sure what is the proper way to fix it, as I'm not that familiar with makefiles. Maybe with .NOTPARALLEL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes apparently a target named .NOTPARALLEL should inhibit the -j cli/config file option

Copy link
Owner

Choose a reason for hiding this comment

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

Cool, that's what I thought. I'm still not sure as to where and how to add it.

e.g., should it be like:

.NOTPARALLEL: downloadffprefs

Or just as it's own line? It wasn't completely clear to me.

Copy link
Contributor Author

@nodiscc nodiscc Apr 3, 2017

Choose a reason for hiding this comment

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

I forgot to post the link I found: http://stackoverflow.com/questions/4346399/how-can-i-force-gnu-make-to-not-build-recipe-in-parallel/23903293#23903293

.PHONY: all clean

.NOTPARALLEL:

anotherTarget: dependency1

Apparently on it's own line. The MAKEFLAGS variable can also be used (solution 3)

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah I saw that post also when I was looking at the subject. Is it so, that everything below that will not run as parallel or what? I mean there are still some things, that can be run as parallel in the makefile, e.g. authors, whatdoesitdo and all the rest for that matter, once downloadffprefs has finished.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it so, that everything below that will not run as parallel or what?

This is what it looks like, but I have no experience with parallel usage of Make - very interesting, I really need to read the full Make manual.

Unfortunately no time to work on it at the moment, would you be ok to apply the global MAKEFLAGS := --jobs=1 var, and open an issue (Makefile: parallelize tests) ? We can improve this later.

Copy link
Owner

Choose a reason for hiding this comment

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

Sounds good.

@pyllyukko
Copy link
Owner

BTW. are you planning on pushing more commits into this? As I'm now tweaking it and trying to get it merged soon.

@nodiscc
Copy link
Contributor Author

nodiscc commented Apr 3, 2017

are you planning on pushing more commits into this

No, please edit it as you like; I will add commit comments on https://github.com/pyllyukko/user.js/commits/nodiscc-fixesfixesfixes if needed, or open other issues.

pyllyukko added a commit that referenced this pull request Apr 3, 2017
pyllyukko added a commit that referenced this pull request Apr 3, 2017
@pyllyukko pyllyukko merged commit c37bf58 into pyllyukko:master Apr 3, 2017
@# generate an AUTHORS file, ordered by number of commits
@# TODO: add a .mailmap file to deduplicate authors with multiple email addresses
@# to add extra authors/credits, git commit --allow-empty --author="A U Thor <author@example.com>"
@git shortlog -sne | cut -f1 --complement >| AUTHORS
Copy link
Owner

@pyllyukko pyllyukko Apr 3, 2017

Choose a reason for hiding this comment

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

BTW. would the following .mailmap file be sufficient for this?

CHEF-KOCH <CHEF-KOCH@users.noreply.github.com> <Nvinside@gmail.com>
Francois Marier <francois@mozilla.com> <francois@debian.org>

Copy link
Contributor Author

@nodiscc nodiscc Apr 4, 2017

Choose a reason for hiding this comment

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

Yes it works with these contents (I just tested it with git shortlog -sne)

Copy link
Owner

Choose a reason for hiding this comment

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

I added it with 563c4c6. Even though I removed the AUTHORS file, but it's still better to get exact stats :)

One thing that is wrong here, is that there are so many people who have contributed through insights in issues & PRs, but haven't pushed any commits.

We would need to have yet-another-script that queries GitHub's API for all the people involved in here to have a complete list :)

Copy link
Contributor Author

@nodiscc nodiscc Apr 4, 2017

Choose a reason for hiding this comment

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

As far as I know there is no way to (directly) list users who posted in a github project issue tracker. But it can be parsed from https://api.github.com/repos/pyllyukko/user.js/issues (login values). If you create a new issue for this, I'll have a look.

Copy link
Owner

Choose a reason for hiding this comment

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

It was a joke, we don't need to do that :)

@pyllyukko pyllyukko mentioned this pull request Apr 4, 2017
7 tasks
@nodiscc nodiscc deleted the fixesfixesfixes branch April 26, 2017 14:48
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.

3 participants