Skip to content

Default the window zoom factor instead of passing NaN - #137

Open
sadiqk2 wants to merge 3 commits into
NativePHP:mainfrom
sadiqk2:fix/window-zoom-factor-nan
Open

Default the window zoom factor instead of passing NaN#137
sadiqk2 wants to merge 3 commits into
NativePHP:mainfrom
sadiqk2:fix/window-zoom-factor-nan

Conversation

@sadiqk2

@sadiqk2 sadiqk2 commented Aug 14, 2026

Copy link
Copy Markdown

window/open destructures zoomFactor from an optional payload and later does:

window.webContents.on('dom-ready', () => {
    window.webContents.setZoomFactor(parseFloat(zoomFactor));
});

When the key is absent that is parseFloat(undefined)NaN, and setZoomFactor(NaN) does not no-op. The page renders at an extreme zoom. I hit this with a window showing about four enormous letters and no other symptom.

Native\Desktop\Windows\Window declares protected float $zoomFactor = 1.0 and always serialises it, so the Laravel client never reaches this path. It only surfaces for a caller that omits the key, which the endpoint otherwise permits.

The fix falls back to 1 for anything non-finite or non-positive.

window/open treats zoomFactor as optional, but parseFloat(undefined) is NaN and
setZoomFactor(NaN) renders the page at an extreme zoom rather than being ignored.

Native\Desktop\Windows\Window declares $zoomFactor = 1.0 and always serialises
it, so the Laravel client never reaches this path — it only shows up for a caller
that omits the key.
@sadiqk2

sadiqk2 commented Aug 17, 2026

Copy link
Copy Markdown
Author

@simonhamp kindly review

@gwleuverink gwleuverink left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find. Confirmed the mechanism: Electron's check is factor < epsilon, and NaN fails that, so it slips straight past the throw and leaves the zoom broken.

The same line sits at /set-zoom-factor (window.ts:95). Could you grab that one while you're in here? A small shared helper would keep the two from drifting.

Both call sites now go through parseZoomFactor(), so a non-numeric or
non-positive value falls back to 1 instead of reaching Electron as NaN.
@sadiqk2

sadiqk2 commented Aug 19, 2026

Copy link
Copy Markdown
Author

Done in c6018b4. Both call sites now go through one helper:

const DEFAULT_ZOOM_FACTOR = 1;

function parseZoomFactor(zoomFactor) {
    const zoom = parseFloat(zoomFactor);

    return Number.isFinite(zoom) && zoom > 0 ? zoom : DEFAULT_ZOOM_FACTOR;
}

/set-zoom-factor (window.ts:95) and the dom-ready handler in /open both call it, so the two cannot drift. Number.isFinite also catches Infinity coming from something like "1e999", and > 0 catches 0 and negatives, which Electron rejects as loudly as it lets NaN through.

I have not rebuilt electron-plugin/dist/. The recent src-only PRs (#123, #114) leave that to the build, so I have kept to the same. tsc --noEmit on the file is clean apart from the pre-existing @types/node noise, and there is no test file for window.ts to extend.

@gwleuverink gwleuverink left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants