1821131 - Exempt a document from more restrictive font visibility rules if it has been exempted from tracking protection

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, enhancement)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

We would like to wire up the ETP permission (the one found in the shield icon, where you can disable ETP for a given site) to the font visibility mechanics. This is to allow a restrictive font visibility to be in place by default, but then allow a user to un-break the page and opt in to a more permissive visibility. This will also apply when the RFP pref is enabled.

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1821131: Exempt a document from stricter font-visibility rules with the ETP toggle r?jfkthame,timhuang

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

I tested this patch manually and it mostly works. The problem is that when I exempt a document and it reloads, it doesn't update the value. It's required for you to close the tab and then open it again. I believe we need to call nsPresContext::UpdateFontVisibility() in the reload flow unconditionally, or call it somewhere in the RTP reload command. The latter seems particularly ugly, but I also couldn't find a good spot in the reload flow where I might insert this call...

It doesn't seem like we have any tests for font visibility, presumably because it's very difficult to test because it requires custom configuration on the machines the test run on.

Comment 3

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1821131: Remove LOAD_FLAGS_MASK r?gijs

Obviously this comment is incorrect, as
LOAD_FLAGS_IS_REDIRECT exists. Additionally, nothing
uses this constant, so just remove it.

Depends on D172041

Comment 4

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1821131: Plumb UpdateFontVisibility into the Reload Path with LOAD_FLAGS r?farre

Depends on D172304

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: WIP: Bug 1821131: A bunch of logging

Depends on D172305

Comment 6

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

So that doesn't work. I've attached patches that plumb everything, but they're actually not necessary to repro the problem, you can repro a variant of it with a clean tree (the second set of steps).

STR:

1. Apply patches
2. Install the JetBrains font
3. Go to https://ritter.vg/misc/ff/font-jetbrains.html and confirm all three are using different fonts. (If they're not, you're going to need to reset prefs or reload the browser or reinstall the font or something)
4. Set layout.css.font-visibility.trackingprotection to 1 and probably restart the browser to be safe
5. Confirm that in regular browser window, you see all three different fonts, and in PBM the middle font matches the bottom.
6. Exempt the page from ETP, it reloads. Same behavior.
7. Copy the URL, open a new tab, paste it, and you should see the middle font in JetBrains now.

That's a page reload. Here's another way to reproduce this where a page should be restyled in-place without a reload, and this happens on a clean tree:

1. Do steps 2-5
2. Edit the pref layout.css.font-visibility.trackingprotection to 3 which should cause UpdateFontVisbility() to be called and a restyle to happen.
3. Observe nothing changed

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1821131 - Make font visibility part of the fontconfig substitution cache key. r=jfkthame

This is one possible option, the other one is to just cache the
substitute results.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Step 1.5: Patch https://searchfox.org/mozilla-central/rev/dbec4165e4c26a0ff970b614842b689e8357593c/gfx/thebes/gfxFcPlatformFontList.cpp#1899 to return ubuntu or fedora.

Comment 9

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Created attachment 9323007 [details]
Bug 1821131 - Make font visibility part of the fontconfig substitution cache key. r=jfkthame

Whoo! I can confirm this fixes the issue (both the ETP opt-out and pref change one). I'm not sure what the Ubuntu/Fedora comment is about though...?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

(In reply to Tom Ritter [:tjr] from comment #9)

Whoo! I can confirm this fixes the issue (both the ETP opt-out and pref change one). I'm not sure what the Ubuntu/Fedora comment is about though...?

I'm just saying that your STR don't work if you're not in Ubuntu or Fedora. I'm on Arch, and I had to hack up that line of code to be able to reproduce the issue since we don't have a "standard font list".

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13a7475298b2
Make font visibility part of the fontconfig substitution cache key. r=jfkthame

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b02c229d8cb
Exempt a document from stricter font-visibility rules with the ETP toggle r=timhuang

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/13a7475298b2

Comment 14

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/7b02c229d8cb

Comment 15

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9321955 [details]
Bug 1821131: Exempt a document from stricter font-visibility rules with the ETP toggle r?jfkthame,timhuang

Beta/Release Uplift Approval Request

• User impact if declined: We will not be able to run experiments in Release
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Both patches are quite small and unlikely to cause issues. They only affect font visibility which is not restricted right now except with RFP enabled.
• String changes made/needed:
• Is Android affected?: No

Comment 16

[Dianna Smith [:diannaS]]

Comment on attachment 9321955 [details]
Bug 1821131: Exempt a document from stricter font-visibility rules with the ETP toggle r?jfkthame,timhuang

Approved for 112.0b6

Comment 17

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/52bca4dd23d3
https://hg.mozilla.org/releases/mozilla-beta/rev/4940c251cc71