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: format change #30

Closed
Thorin-Oakenpants opened this issue Feb 25, 2017 · 10 comments
Closed

ToDo: format change #30

Thorin-Oakenpants opened this issue Feb 25, 2017 · 10 comments

Comments

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Feb 25, 2017

Just using a new issue to deal with this rather than OT elsewhere.

Here is a pastebin which expires in 24hrs.

Because every single non user_pref line is modified, I have taken the opportunity to do more than that. All up:

  • formatted as discussed earlier which creates foldable preference numbers
  • where a numbered item had user_pref lines punctuated by comments, I did something about it (eg all the current 3021 items) so that every single foldable item has a number
  • acronyms are expanded eg DHE, DRM, CDM etc where needed
  • all version notations are now (xx) for deprecated and (FFxx) for introduced
  • removed all instances of "i" and "author" and used 1st person plural (if indeed even needed anymore)
  • removed ghacks html tags
  • reviewed/checked all warning tags (got rid of some) and added setup tags: they now use [WARNING] and [SETUP]
  • removed all the rubbish from the doc header section and rewrote a quick readme to make some things painfully clear. PS: the readme is not done, but the little readme in the user.js covers all the really important stuff, is seriously short, simple, and easy to read
  • did away with the troubleshooting list, you'll see why (hated maintaining that list)
  • those painfully clear things are also made painfully clear in those sections or preferences, some rewording.
  • a few descriptions were slightly reworded, or two liners moved into one
  • made a few other words/phrases etc uniform
  • went over it a half dozen times

The idea, without going overboard, with the [WARNING] and [SETUP] tags is that warning is more about (major) site breaking etc and setup is stuff that is optional or should be looked at when setting up - doh! By that, I mean that 90% of the 270+ numbered items cause zero breakage or issues, 5% are optional i.e the user can make an informed decision and knows the drawbacks (eg keeping history), and the rest are items that cause site breakage or are bad to implement etc. The two tags can be used in conjunction (i.e more than one tag per item), they can be at the end of a section title, at the end of a preference number title, in a comment at the end of a user_pref line, in the middle of a section description, or at the start of a item's warning section etc.

@earthlng I have tested it for syntax, etc. Do one of your compares to make sure no pref values got changed, or prefs switched between active/inactive. And then make any changes you want and pastebin me back a version. Make any changes you want (ignore passwords/referrers sections), because this is a one time deal since 70% of the lines are going to change. Take your time.

@everyone else - if you want to proof-read it when you have some time, or offer suggestions, go for it. I can do the commit when we're all happy.

@Just-me-ghacks
Copy link

Just-me-ghacks commented Feb 25, 2017

  • replace "Covenience" with "Convenience"
  • replace "surpises" with "surprises"
  • replace "shoulder surfers,forensics" with "shoulder surfers, forensics"
  • remove double space "disable URLbar autofill - PRIVACY"
  • "Try it You are not missing" - you?
  • and impacts video imperformance ???
  • replace "signifcant" with "significant"
  • remove the space at the end of line 976.
  • Facebook but youtube, microsoft - why capital F?
    • Roman says: cheers, I need to use Capital for when talking about them as a company but lowercase as a website
  • capital letter? "then also make sure 0806"
    • Roman says: not sure what you mean, but URLbar was probably written like to emphasize it was the URl bar and not some other bar, its now just urlbar all lower case, if that was what you were referring to

@earthlng
Copy link
Contributor

http://pasted.co/5c0cfcc1

Only 1 change from my comparison: 2703 was commented out in master, active in your pastebin

Apart from a few other things, I changed the numbers under 0410, not a fan of things like 0410b2, but feel free to change it back or do a mix , or whatever you think is best

some of the things Just Me noticed are corrected in my pastebin, but not everything. (only the first 2 I think)

@Just-me-ghacks
Copy link

Just-me-ghacks commented Feb 25, 2017

capital letter? "then also make sure 0806" - My bad, ignore it. I didn't see the comma before "then".

  • disable dns prefetching - disable DNS prefetching
  • dns leak issue - DNS leak issue
  • disable referer from - disable referrer from
    • no change. referrer vs referer .. there are 3 of the former and they are site links or a pref name, everything else is the later and there are 10 of them. The correct word is a single r
  • 2001: disable WebRTC - expand to "WebRTC (Web Real-Time Communication)"
  • 2010: disable WebGL - expand to "WebGL (Web Graphics Library)"
  • "and fear of breakage/bugs ***/" - remove double space

Roman says: use

* [ ] comment

and then I can just tick rather than edit

@earthlng
Copy link
Contributor

earthlng commented Feb 25, 2017

IMO we should always use 0410a, b, c, etc. for "meta"-numbers. It allows for more than just 041[1-9] prefs in one group.

EDIT: fe. 0420 could be SB + TP "description only" number, 0421 SB, 0422 TP and 0415 would become 0423 because it applies to both. We would have SB + TP all under 042x that way.
Sorry, a bit late with the suggestions but I mainly looked at other things during the comparing.

@Just-me-ghacks
Copy link

Just-me-ghacks commented Feb 25, 2017

  • replace ` with '
    • initially started putting pref names inside ` or ' but stopped, too much going on. All of the former are now removed
  • 2671: disable SVG - expand to "SVG (Scalable Vector Graphics)"
  • 2663: disable MathML - expand to "MathML (Mathematical Markup Language)"

@earthlng
Copy link
Contributor

earthlng commented Feb 25, 2017

maybe slightly OT, but hear me out ;)

I still like the idea of making this overall more modular by using a preferences folder.
But we would only use a separate file IF we can really isolate certain prefs for a certain feature, SB + TP come to mind.
We could release with safebrowsing-trackerprotection.js.prefs and without needing to edit anything, users could just rename the file to disable SB + TP.
Since you plan to enable SB+TP by default anyway, this would be a neat way to handle it.
Of course this would only work when we want to ship with a feature completely on it's default values.

#32 (comment) Restore video settings

If we had multiple files, we could consider using 'pref' instead of 'user_pref' for certain files (fe media)
I know it has a lot of downsides, but one major advantage is that the file could simply be renamed or deleted and the original defaults would be restored.

@earthlng
Copy link
Contributor

earthlng commented Feb 25, 2017

Give it one last pass if you can :)

  • 0410a - F48 instead of FF48 (?)
  • 0410b - exists twice
  • 2672 - move the "than sorry" to the previous line. it's still shorter than the title line
  • 3021b - why do you insist on removing the line [NOTE] does not apply to middle-click or Ctrl-clicking links. ? imo that's nice to know
  • 3021c - no space between /* and number

IMO it's always good to have an empty newline at EOF

@Just-me-ghacks
Copy link

Just-me-ghacks commented Feb 26, 2017

  • 0410d: disable mozilla safebrowsing // mozilla or Mozilla?
    (0410c: disable Google safebrowsing & 0440: disable Mozilla's blocklist)
  • 0810: disable css querying page history - css history leak // CSS?
  • 1405: disable woff2 - expand to WOFF (Web Open Font Format)
    • Roman says, its WOFF version 2
  • This will break some sites eg Dropbox, Google Docs? gmail? // capital letters inconsistency?
  • Roboform // RoboForm
    • Roman says: can't even find roboform or the other one mentioned at ghacks (Internet Download Manager) on AMO.
  • lastest ESR // latest
  • site funtionality // functionality

@Thorin-Oakenpants - I like your writing style and the original user.js. Please don't approve everything and don't change things just for the sake of changing something. Just saying...

@earthlng
Copy link
Contributor

earthlng commented Feb 26, 2017

  • 1200 - start the sentence after the URL on a new line
  • 1208 - 0=disabled instead of 0-disabled
  • 1215 - should be 0 = blabla, 1 = blabla -- same as 1211. plus add (FF50+) note
  • browser.usedOnWindows10.introURL // removed in FF50 (was in my diff) => 0 results in DXR for 'usedOnWindows10'
  • 2617 - says disable in title but enables it
  • add comment // Form & Search History to privacy.clearOnShutdown.formdata
  • uppercase first letter // Active Logins for privacy.clearOnShutdown.sessions
  • better title for 1200 fe. HTTPS ( SSL / OCSP / CERTS / ENCRYPTION / HSTS / HPKP )
  • 1222 doesn't fit in 1200, has nothing to do with HTTPS (2600?)
  • 2613 is HW fingerprinting and would fit better under 2500
  • 2671: change title to disable in-content SVG (Scalable Vector Graphics) (FF53+)
  • 1401: title is incorrect, it's not about "downloading fonts"

==Roman comments added==

  • maybe move 0430 to 1200
    • wontfix: : I think it fits nicely with keeping ff quiet, and also 400 is about blocklists, revoked certs .. and reporting certs is part of that. Whereas 1200 is more about what certs etc to use. I will leave where it is.
  • 3021b - I think it applies when you close a tab with middle-clicking the tab
    • wontfix: Roman says: we have lots of other click related preferences .. I don't think we should have to state on each one that key+mouse combos may do different behavior. When I was editing, that one on 3021b to me just looked out of place. So out it goes.
  • rename 2660 to 2653 -> e10s
    • wontfix for now: this can be dealt with when all the e10s stuff is worked out
  • IMO we should make 2700 only for Cookies, ie. move 2707 to cache (?), and 2705+2706 to 2400
    • wontfix on this ticket. I agree 2707 could move, the whole section is a tad messy. cookies on their own don't warrant a section. I tried to add dom storage since they seem to be handled together by a lot of addons. Create a ticket for discussion.

--WTF

I'm sorry, but you said make any changes you want ;)

Thorin-Oakenpants pushed a commit that referenced this issue Feb 27, 2017
#30
no preference value changes or active/inactive status, just descriptions and formatting
@earthlng
Copy link
Contributor

earthlng commented Feb 27, 2017

testsite: you need to enable layout.css.visited_links_enabled and test it in a Normal Window. PB doesn't seem to work. Check the source and visit a few of the URLs from the list and then start the test. It uses a lot of http sites instead of https, so it can't detect some of the sites anymore because they moved to https.
But I did the test and apart from one, it correctly identified all other sites I visited (fe. didn't detect http://reddit.com/r/conspiracy, because that's https nowadays)

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