Closed Bug 1501985 Opened 6 years ago Closed 5 years ago

Update Content Blocking section in about:preferences

Categories

(Firefox :: Settings UI, enhancement, P2)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 65
Tracking Status
firefox65 --- verified

People

(Reporter: johannh, Assigned: ewright)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [privacy65])

Attachments

(1 file)

We have new UX specs for 65: https://mozilla.invisionapp.com/share/RHORBWDNX4W#/screens

Things we should do in this bug (or in sub-bugs):

- Remove the "Restore Defaults" button
- Add a large shield icon
- Add a card-like UI that allows the user to switch between "Standard", "Strict" and "Custom" mode
Depends on: 1501977
Assignee: nobody → ewright
Status: NEW → ASSIGNED
I made some builds for Mac, Windows and Linux if anyone would like to try the progress so far.
cc: @bbell

https://treeherder.mozilla.org/#/jobs?repo=try&revision=682af25fdf11652b6a96dfd4be3a0ed9a9b95ea0
Flags: needinfo?(bbell)
I played with the Linux build for a short while and it looks really good, great job!  One tiny issue that caught my eyes was that the up arrow icon doesn't show up for the expander buttons on each section.

Also when testing RTL mode (using https://firefox-source-docs.mozilla.org/intl/l10n/l10n/fluent_tutorial.html#pseudolocalization) I noticed that the margins around the icons under each subsection listing the blocked features seem to not get applied in RTL mode.
Wow, this looks great already. One thing I immediately noticed when playing around with it on OSX is that the sections don't expand when you click on the description, but only when you click outside of the description or on the radio button. I'm not sure if that's intended, I found it a bit annoying.

I didn't check in detail whether the different pref states were accurately changed by the menu, I guess that can happen during the actual code review :)
I've posted the code on Phabricator - it's not quite ready for review yet. I mostly posted this so I could get a better picture of the changes, feel free to take a look, and o=point out if I've gone in the wrong direction somewhere.
Todo: 
- test with screen reader and ensure all aria labels are restored.
- double check the i20n labels
- currently working on fixing tests
- rename some functions
This adds a card-like UI to the content blocking section in preferences.
Pushed by ewright@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6a9f15fb809d
Update Content Blocking section UI r=flod,johannh
https://hg.mozilla.org/mozilla-central/rev/6a9f15fb809d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Depends on: 1508562
Depends on: 1508712
Depends on: 1508716
Depends on: 1508806
Depends on: 1509411
I have reproduced this bug with Nightly 65.0a1 (2018-10-25) on Windows 7, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID 	20181122220059
User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0
QA Whiteboard: [bugday-20181121]
This bug seems to change how strings for trackers radio group and cookies dropdown menulist are used.

That itself is against https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Don't_reuse_strings_in_different_contexts (radio labels changed to menulist items, menulist in completely different grammar context) but there is even more important problem. Because l10n strings are shared between Firefox versions I don't think it will be possible to adapt existing translations to new and old versions of UI.

Affected strings need to be updated unless the change is meant to land in 64 shortly.
Flags: needinfo?(ewright)
(In reply to Stefan Plewako [:stef] from comment #10)
> This bug seems to change how strings for trackers radio group and cookies
> dropdown menulist are used.
> 
> That itself is against
> https://developer.mozilla.org/en-US/docs/Mozilla/Localization/
> Localization_content_best_practices#Don't_reuse_strings_in_different_contexts
> (radio labels changed to menulist items, menulist in completely different
> grammar context) but there is even more important problem. Because l10n
> strings are shared between Firefox versions I don't think it will be
> possible to adapt existing translations to new and old versions of UI.
> 
> Affected strings need to be updated unless the change is meant to land in 64
> shortly.

:stef I think I see what you mean, I didn't think of that. I'll update the string ids of the strings that changed context.
Flags: needinfo?(ewright)
That would be great, thank you!

One more: content-blocking-tracking-protection-trackers-label
Depends on: 1509936
Verified - Fixed on latest Nightly 65.0a1 (2018-11-28) (64-bit) on Windows 7/10 x64, Ubuntu 18.04. and Mac OS 10.14.
Status: RESOLVED → VERIFIED
See Also: → 1518079
Depends on: 1533182
Regressions: 1544480
No longer regressions: 1544480
Assignee: ewright → nobody
Assignee: nobody → ewright
Depends on: 1552773
Flags: needinfo?(bbell)
You need to log in before you can comment on or make changes to this bug.