Make system fonts work more reliably (#8602)

* Make system fonts work more reliably

* Make it more sophisticated

* Missing semicolon

* Apply suggestions

* Fix formatting

Co-authored-by: Michael Telatynski <[email protected]>

* Create FontWatcher-test.tsx

* Add actual tests

* Fix some errors

* Apply suggestions

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

* Fix FontWatcher tests

* Correct test fixture

Co-authored-by: Michael Telatynski <[email protected]>

Author: r00ster

src/settings/watchers/FontWatcher.ts

@@ -64,6 +64,20 @@ export class FontWatcher implements IWatcher {
     };
 
     private setSystemFont = ({ useSystemFont, font }) => {
-        document.body.style.fontFamily = useSystemFont ? font : "";
+        if (useSystemFont) {
+            // Make sure that fonts with spaces in their names get interpreted properly
+            document.body.style.fontFamily = font
+                .split(',')
+                .map(font => {
+                    font = font.trim();
+                    if (!font.startsWith('"') && !font.endsWith('"')) {
+                        font = `"${font}"`;
+                    }
+                    return font;
+                })
+                .join(',');
+        } else {
+            document.body.style.fontFamily = "";
+        }
     };
 }

test/CallHandler-test.ts

@@ -21,12 +21,10 @@ import EventEmitter from 'events';
 import CallHandler, {
     CallHandlerEvent, PROTOCOL_PSTN, PROTOCOL_PSTN_PREFIXED, PROTOCOL_SIP_NATIVE, PROTOCOL_SIP_VIRTUAL,
 } from '../src/CallHandler';
-import { stubClient, mkStubRoom } from './test-utils';
+import { stubClient, mkStubRoom, untilDispatch } from './test-utils';
 import { MatrixClientPeg } from '../src/MatrixClientPeg';
-import dis from '../src/dispatcher/dispatcher';
 import DMRoomMap from '../src/utils/DMRoomMap';
 import SdkConfig from '../src/SdkConfig';
-import { ActionPayload } from '../src/dispatcher/payloads';
 import { Action } from "../src/dispatcher/actions";
 
 // The Matrix IDs that the user sees when talking to Alice & Bob
@@ -95,18 +93,6 @@ class FakeCall extends EventEmitter {
     }
 }
 
-function untilDispatch(waitForAction: string): Promise<ActionPayload> {
-    let dispatchHandle;
-    return new Promise<ActionPayload>(resolve => {
-        dispatchHandle = dis.register(payload => {
-            if (payload.action === waitForAction) {
-                dis.unregister(dispatchHandle);
-                resolve(payload);
-            }
-        });
-    });
-}
-
 function untilCallHandlerEvent(callHandler: CallHandler, event: CallHandlerEvent): Promise<void> {
     return new Promise<void>((resolve) => {
         callHandler.addListener(event, () => {

test/settings/watchers/FontWatcher-test.tsx

@@ -0,0 +1,54 @@
+/*
+Copyright 2022 r00ster91 <[email protected]>
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { sleep } from 'matrix-js-sdk/src/utils';
+
+import SettingsStore from '../../../src/settings/SettingsStore';
+import { SettingLevel } from '../../../src/settings/SettingLevel';
+import { FontWatcher } from "../../../src/settings/watchers/FontWatcher";
+import { Action } from "../../../src/dispatcher/actions";
+import { untilDispatch } from "../../test-utils";
+
+async function setSystemFont(font: string): Promise<void> {
+    await SettingsStore.setValue("systemFont", null, SettingLevel.DEVICE, font);
+    await untilDispatch(Action.UpdateSystemFont);
+    await sleep(1); // await the FontWatcher doing its action
+}
+
+describe('FontWatcher', function() {
+    let fontWatcher: FontWatcher;
+    beforeEach(() => {
+        fontWatcher = new FontWatcher();
+        fontWatcher.start();
+        return SettingsStore.setValue("useSystemFont", null, SettingLevel.DEVICE, true);
+    });
+    afterEach(() => {
+        fontWatcher.stop();
+    });
+
+    it('encloses the fonts by double quotes and sets them as the system font', async () => {
+        await setSystemFont("Fira Sans Thin, Commodore 64");
+        expect(document.body.style.fontFamily).toBe(`"Fira Sans Thin","Commodore 64"`);
+    });
+    it('does not add double quotes if already present and sets the font as the system font', async () => {
+        await setSystemFont(`"Commodore 64"`);
+        expect(document.body.style.fontFamily).toBe(`"Commodore 64"`);
+    });
+    it('trims whitespace, encloses the fonts by double quotes, and sets them as the system font', async () => {
+        await setSystemFont(`  Fira Code  ,  "Commodore 64" `);
+        expect(document.body.style.fontFamily).toBe(`"Fira Code","Commodore 64"`);
+    });
+});

test/test-utils/utilities.ts

@@ -17,8 +17,24 @@ limitations under the License.
 import { ReactWrapper } from "enzyme";
 import EventEmitter from "events";
 
+import { ActionPayload } from "../../src/dispatcher/payloads";
+import defaultDispatcher from "../../src/dispatcher/dispatcher";
+import { DispatcherAction } from "../../src/dispatcher/actions";
+
 export const emitPromise = (e: EventEmitter, k: string | symbol) => new Promise(r => e.once(k, r));
 
+export function untilDispatch(waitForAction: DispatcherAction): Promise<ActionPayload> {
+    let dispatchHandle: string;
+    return new Promise<ActionPayload>(resolve => {
+        dispatchHandle = defaultDispatcher.register(payload => {
+            if (payload.action === waitForAction) {
+                defaultDispatcher.unregister(dispatchHandle);
+                resolve(payload);
+            }
+        });
+    });
+}
+
 const findByAttr = (attr: string) => (component: ReactWrapper, value: string) => component.find(`[${attr}="${value}"]`);
 export const findByTestId = findByAttr('data-test-id');
 export const findById = findByAttr('id');