Skip to content

Commit

Permalink
Cleanup to resolve some PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
djahandarie committed Dec 22, 2024
1 parent 3622197 commit ec5607b
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 13 deletions.
6 changes: 4 additions & 2 deletions ext/js/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,15 @@ export class Application extends EventDispatcher {
* @param {(application: Application) => (Promise<void>)} mainFunction
*/
static async main(waitForDom, mainFunction) {
const supportsServiceWorker = 'serviceWorker' in navigator; // Basically, all browsers except Firefox. But it's possible Firefox will support it in the future, so we check in this fashion to be future-proof.
const inExtensionContext = window.location.protocol === new URL(import.meta.url).protocol; // This code runs both in content script as well as in the iframe, so we need to differentiate the situation
/** @type {MessagePort | null} */
// If this is Firefox, we don't have a service worker and can't postMessage,
// so we temporarily create a SharedWorker in order to establish a MessageChannel
// which we can use to postMessage with the backend.
// This can only be done in the extension context (aka iframe within popup),
// not in the content script context.
const backendPort = (!('serviceWorker' in navigator)) && window.location.protocol === new URL(import.meta.url).protocol ?
const backendPort = !supportsServiceWorker && inExtensionContext ?
(() => {
const sharedWorkerBridge = new SharedWorker(new URL('comm/shared-worker-bridge.js', import.meta.url), {type: 'module'});
const backendChannel = new MessageChannel();
Expand All @@ -210,7 +212,7 @@ export class Application extends EventDispatcher {
log.configure(webExtension.extensionName);

const mediaDrawingWorkerToBackendChannel = new MessageChannel();
const mediaDrawingWorker = window.location.protocol === new URL(import.meta.url).protocol ? new Worker(new URL('display/media-drawing-worker.js', import.meta.url), {type: 'module'}) : null;
const mediaDrawingWorker = inExtensionContext ? new Worker(new URL('display/media-drawing-worker.js', import.meta.url), {type: 'module'}) : null;
mediaDrawingWorker?.postMessage({action: 'connectToDatabaseWorker'}, [mediaDrawingWorkerToBackendChannel.port2]);

const api = new API(webExtension, mediaDrawingWorker, backendPort);
Expand Down
6 changes: 3 additions & 3 deletions ext/js/comm/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,9 @@ export class API {
}
this._backendPort.postMessage({action, params}, transferables);
} else {
void navigator.serviceWorker.ready.then((swr) => {
if (swr.active !== null) {
swr.active.postMessage({action, params}, transferables);
void navigator.serviceWorker.ready.then((serviceWorkerRegistration) => {
if (serviceWorkerRegistration.active !== null) {
serviceWorkerRegistration.active.postMessage({action, params}, transferables);
} else {
log.error(`[${self.constructor.name}] no active service worker`);
}
Expand Down
4 changes: 3 additions & 1 deletion ext/js/comm/shared-worker-bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import {createApiMap, invokeApiMapHandler} from '../core/api-map.js';
import {log} from '../core/log.js';

/**
* This serves as a bridge between the application and the backend.
* This serves as a bridge between the application and the backend on Firefox
* where we don't have service workers.
*
* It is designed to have extremely short lifetime on the application side,
* as otherwise it will stay alive across extension updates (which only restart
* the backend) which can lead to extremely difficult to debug situations where
Expand Down
5 changes: 3 additions & 2 deletions ext/js/dictionary/dictionary-database-worker-handler.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/*
* Copyright (C) 2023-2024 Yomitan Authors
* Copyright (C) 2021-2022 Yomichan Authors
* Copyright (C) 2024 Yomitan Authors
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -48,6 +47,8 @@ export class DictionaryDatabaseWorkerHandler {
case 'connectToDatabaseWorker':
void this._dictionaryDatabase?.connectToDatabaseWorker(event.ports[0]);
break;
default:
log.error(`Unknown action: ${action}`);
}
}
}
3 changes: 1 addition & 2 deletions ext/js/dictionary/dictionary-database-worker-main.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/*
* Copyright (C) 2023-2024 Yomitan Authors
* Copyright (C) 2021-2022 Yomichan Authors
* Copyright (C) 2024 Yomitan Authors
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down
8 changes: 5 additions & 3 deletions ext/js/display/structured-content-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,16 @@ export class StructuredContentGenerator {
/** @type {HTMLCanvasElement} */ (this._createElement('canvas', 'gloss-image')) :
/** @type {HTMLImageElement} */ (this._createElement('img', 'gloss-image'));
if (sizeUnits === 'em' && (hasPreferredWidth || hasPreferredHeight)) {
const emSize = 14; // We could Number.parseFloat(getComputedStyle(document.documentElement).fontSize); here for more accuracy but it would cause a layout and be extremely slow; possible improvement would be to calculate and cache the value
const scaleFactor = 2 * window.devicePixelRatio;
image.style.width = `${usedWidth}em`;
image.style.height = `${usedWidth * invAspectRatio}em`;
image.width = usedWidth * 14 * 2 * window.devicePixelRatio;
image.height = usedWidth * invAspectRatio * 14 * 2 * window.devicePixelRatio;
image.width = usedWidth * emSize * scaleFactor;
} else {
image.width = usedWidth;
image.height = usedWidth * invAspectRatio;
}
image.height = image.width * invAspectRatio;

imageContainer.appendChild(image);

if (this._contentManager instanceof DisplayContentManager) {
Expand Down

0 comments on commit ec5607b

Please sign in to comment.