59 bytes, text/x-review-board-request
On Mac, many users use 3rd party font managers which can store fonts on the filesystem anywhere the user chooses. We should have a test that installs a font from a non-standard directory and then renders a page using that font, making sure the font is displayed correctly.
Assignee: nobody → haftandilian
Priority: -- → P2
A CLI command that registers or unregisters fonts in place. Using this command, we could register a font from a non-standard location, display a page that depends on this font, validate the page is displayed correctly, then uninstall the font. Any directory not included in our content sandbox read-access whitelist is considered a non-standard font directory. For example, ~/MyFonts/font.otf.
Register/unregister fonts in python to make this easier to use in automated tests--no compilation required.
Work-in-progress patch that adds a test that checks that a page using the Fira Sans font displays correctly when the font is installed in a standard directory readable from content processes AND a directory not readable from content processes. Fira Sans is not included with macOS but is already in the tree. The test passes locally on 10.13, but fails on try with 10.10. In addition to the try server failure, some synchronization and other improvements are still needed.
Attachment #8944874 - Attachment is obsolete: true
In debugging the 10.10 try failure referenced in comment 4, I found that the font, when installed in a private directory (that content doesn't have read access to) is picked up by content and the sandbox extension mechanism is working. I have not yet confirmed why the test fails. It may be due to a race condition in how the test waits for a font to be installed before testing it is displayed by content. The test uses a prefObserver with "font.internaluseonly.changed" to wait until installing the font in the OS is detected by the browser, but the pref notification occurs in the parent and test doesn't wait for any indication the new font has been propagated to the child. Needs more debugging.
With help from Jonathan, I've updated the patch to not use snapshots but instead to check for the LastResort font being used. The test registers a font from the root of the home directory which is not normally accessible to content processes. As a follow-up, the test could be updated to test all the cases we are currently whitelisting in SandboxPolicies.h for 10.11 and earlier.
Attachment #8979155 - Attachment is obsolete: true
Looks reasonable. Just to check, have you verified that the test actually fails on a revision prior to bug 1460917?
(In reply to Jonathan Kew (:jfkthame) from comment #10) > Looks reasonable. Just to check, have you verified that the test actually > fails on a revision prior to bug 1460917? I tested locally but didn't think to submit that to try. I submitted one this morning with the fix for bug 1460917 backed out and it fails in the right way (because the try builds run on 10.10 which is affected by bug 1460917): TEST-UNEXPECTED-FAIL | security/sandbox/test/browser_bug1393259.js | The LastResort fallback font was not used - false == true - JS frame :: chrome://mochitests/content/browser/security/sandbox/test/browser_bug1393259.js :: <TOP_LEVEL> :: line 160 TEST-UNEXPECTED-FAIL | security/sandbox/test/browser_bug1393259.js | The test font "Fira Sans" was used - false == true - JS frame :: chrome://mochitests/content/browser/security/sandbox/test/browser_bug1393259.js :: <TOP_LEVEL> :: line 163 https://treeherder.mozilla.org/#/jobs?repo=try&revision=78d03dd107fd01f38cb7860fe2c5e05fe35b2e15
Comment on attachment 8979742 [details] Bug 1432567 - [Mac] Add a test that renders fonts from non-standard directories https://reviewboard.mozilla.org/r/245888/#review252582 ::: security/sandbox/test/browser.ini:11 (Diff revision 1) > + mac_register_font.py > + ../../../layout/reftests/fonts/fira/FiraSans-Regular.otf > > -skip-if = !e10s > [browser_content_sandbox_fs.js] > -skip-if = !e10s || (debug && os == 'win') # bug 1379635 > +skip-if = !e10s The "skip-if (debug && os=='win')" needs to be applied to browser_content_sandbox_fs.js and my changes broke this. Will fix.
Comment on attachment 8979742 [details] Bug 1432567 - [Mac] Add a test that renders fonts from non-standard directories https://reviewboard.mozilla.org/r/245888/#review252600 OK, thanks for checking it out on tryserver. LGTM, with the above issue fixed.
Attachment #8979742 - Flags: review?(jfkthame) → review+
Comment on attachment 8979742 [details] Bug 1432567 - [Mac] Add a test that renders fonts from non-standard directories https://reviewboard.mozilla.org/r/245888/#review252600 Thanks for the review.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/c869e76b79c5 [Mac] Add a test that renders fonts from non-standard directories r=jfkthame
You need to log in before you can comment on or make changes to this bug.