Skip to content

feat: modernize platform/device detection + Application fullscreen API (#1467)#1485

Open
obiot wants to merge 14 commits into
masterfrom
fix/1467-platform-detection-modernize
Open

feat: modernize platform/device detection + Application fullscreen API (#1467)#1485
obiot wants to merge 14 commits into
masterfrom
fix/1467-platform-detection-modernize

Conversation

@obiot
Copy link
Copy Markdown
Member

@obiot obiot commented Jun 1, 2026

Closes #1467.

Scope grew during review from the original "iPadOS 13+ detection + dead-platform deprecation" into a broader platform/device cleanup. Updated to reflect what actually ships.

What this fixes / changes

1. Modern iPads (iPadOS 13+, since Sept 2019) report as desktop

Safari on iPad ships the desktop Mac UA — no iPad token. The /iPhone|iPad|iPod/i regex missed every iPad sold in the last ~7 years; they all fell through isMobile as desktop. Every internal consumer (keyboard.ts, application.ts, header.ts) was getting the wrong answer for ~30% of mobile traffic.

Fix: layer a feature check on top of the UA regex:

const isIPadOnMacUA =
    navigator?.platform === "MacIntel" && (navigator?.maxTouchPoints ?? 0) > 1;
export const iOS = /iPhone|iPad|iPod/i.test(ua) || isIPadOnMacUA(_nav);

navigator.platform === "MacIntel" is NOT a CPU check — Apple deliberately freezes the legacy string for backwards compat (same trick Windows uses with Win32 on 64-bit). Apple Silicon Macs / iPads (M1, M2, M3, M4) all report MacIntel. The maxTouchPoints > 1 clause is what actually separates iPads from real Macs.

isIPadOnMacUA(nav) is extracted as a @internal-tagged exported function so the spec asserts the SAME predicate the module evaluates at load time (drift-proof). Marked @internal because it's a test-seam, not stable API.

2. Dead-platform regex noise

@deprecated since 19.7.0 on wp / BlackBerry / Kindle / android2. Exports stay functional through 19.x for backwards compat; IDE warnings light up at call sites. Removal scheduled for 20.x.

Also dropped these from isMobile's OR chain — remaining /Mobi/.test(ua) || iOS || android covers ~99.9% of 2026 mobile traffic per MDN.

3. system/device.jssystem/device.ts (945 lines)

Mechanical conversion — JSDoc was already exhaustive, so most types just become native TS annotations. Non-standard / legacy browser surfaces (Document.mozFullScreenEnabled, Navigator.standalone, iOS-only DeviceOrientationEvent.requestPermission, etc.) typed via narrow local intersection types. Two small correctness improvements that fell out:

  • onDeviceMotion now guards against accelerationIncludingGravity === null
  • getElement JSDoc corrected (function never returns null, always falls back to document.body)

4. Application#requestFullscreen / exitFullscreen / isFullscreen — fullscreen finally has app-instance context

device.requestFullscreen() had to fall back to the deprecated getParent()game.getParentElement() global-game lookup because the static helper had no Application reference. The new methods on Application use this.parentElement directly — canvas + sibling HUD go fullscreen together, no deprecated chain.

device.requestFullscreen / device.exitFullscreen deprecated (since 19.7.0) pointing at the Application methods. device.isFullscreen stays non-deprecated because it's a stateless document-state probe (there's exactly one fullscreen state per document regardless of how many Applications run); Application#isFullscreen is a thin convenience.

Also cleaned up the underlying probe in the deprecated wrappers — replaced prefixed("fullscreenElement", document as unknown as Record<string, unknown>) etc. with an explicit four-variant OR chain (the pattern lib.dom.d.ts itself uses).

5. keyboard.ts — drop the if (!isMobile) gate

Initially marked as a follow-up but pulled into this PR after the iPad fix made the gate observably wrong. The gate assumed "mobile = no physical keyboard" — invalid for iPads (now correctly identified) with Magic Keyboard, Samsung DeX, ChromeOS tablet mode, Bluetooth-keyboard-on-phone. Two empty listener slots cost nothing on touch-only devices; the unbound-key path is a single map lookup that returns undefined.

Also changed the surrounding if (globalThis.addEventListener) to typeof globalThis.addEventListener === "function" per Copilot's defensive-style suggestion.

6. Example migrations

  • platformer/createGame.ts, platformer/HUD.ts, platformer-matter/createGame.ts, platformer-matter/HUD.ts migrated from device.{is,request,exit}Fullscreen() to app.* / game.* (canonical post-19.7 API).
  • afterBurner/ExampleAfterBurner.tsx — added F→fullscreen shortcut + canvas retuned 1024×768 → 1024×576 (16:9, suits widescreen fullscreen better).
  • afterBurner/HUD.ts — refactored to read live app.viewport.width / .height so HUD positions follow the configured Application size (was previously hardcoded to the old 1024×768). Added "Powered by melonJS" credit and tightened the bottom credit strip.

Won't-add: isTouch

Original issue suggested a new isTouch flag. We already have device.touch at system/device.ts — feature-detected via navigator.maxTouchPoints / pointer events. CHANGELOG migration note points there.

Tests

Six new cases in tests/platform.spec.ts covering the documented iPad-detection contract:

  • ✅ Apple Silicon iPad reporting as Mac (platform=MacIntel, maxTouchPoints=5) → flagged
  • ✅ Actual Mac (platform=MacIntel, maxTouchPoints=0) → not flagged
  • ✅ Mac with maxTouchPoints undefined (older Safari) → not flagged
  • ✅ Windows touchscreen (platform=Win32, maxTouchPoints=10) → not flagged
  • ✅ Missing navigator (Node/SSR) → not flagged
  • maxTouchPoints === 1 (single-point touch edge case) → not flagged (> 1 excludes it)

Existing 20 shape / desktop-defaults assertions kept.

Test plan

  • pnpm test:types clean
  • pnpm vitest run — 3975 / 13 skipped / 0 failed (was 3969, +6 new)
  • pnpm build — lint + types clean
  • AfterBurner F→fullscreen verified in dev server (1024×576 canvas, post-build dynamic HUD)

Follow-ups (separate tickets)

Filed out of scope:

🤖 Generated with Claude Code

Two real problems in `src/system/platform.ts` flagged during the
19.7 audit:

1. Modern iPads (iPadOS 13+, since Sept 2019) ship Safari with the
   desktop Mac UA — no `iPad` token in the user-agent string. The
   `/iPhone|iPad|iPod/i` test missed every iPad sold in the last
   ~7 years, and they fell through `isMobile` as desktop. Confirmed
   observable: `keyboard.ts`, `application.ts`, `header.ts` all
   branch on `isMobile`.

2. Dead-platform UA regexes (`wp`, `BlackBerry`, `Kindle`, `android2`)
   tested for hardware that was EOL'd between 2012 and 2017, burning
   regex cycles on every page load.

## Changes

**iPad detection**: layer a feature check on top of the UA regex —
`navigator.platform === "MacIntel"` plus `maxTouchPoints > 1`. The
`"MacIntel"` string is Apple's frozen legacy identifier (same trick
as `Win32` on 64-bit Windows) that persists on Apple Silicon Macs
*and* iPads in desktop-Safari mode — it's not a CPU check. `Macs
don't have touchscreens; iPads do`, so `maxTouchPoints > 1`
uniquely separates them.

Every existing internal consumer of `isMobile` inherits the fix
transparently.

**Deprecate dead exports**: `@deprecated` JSDoc on `wp`, `BlackBerry`,
`Kindle`, `android2`. Exports stay functional through 19.x for
backwards compat (any external consumer keeps working); IDE warnings
surface at the call sites. Removal scheduled for 20.x.

Also dropped these from `isMobile`'s OR chain. The remaining
`/Mobi/.test(ua) || iOS || android` covers ~99.9% of 2026 mobile
traffic per MDN's recommendation.

**Won't add `isTouch`** as the original issue suggested — we already
have `device.touch` at `system/device.js:116` (feature-detected via
`navigator.maxTouchPoints` / pointer events). CHANGELOG migration
note points there for new code.

## Tests

Six new cases in `tests/platform.spec.ts` covering the iPad-on-Mac-UA
contract — verify the documented `platform === "MacIntel" &&
maxTouchPoints > 1` check identifies iPads correctly, rejects actual
Macs (no touch), Windows touchscreens, and missing-navigator (SSR).
Existing 20 shape / desktop-defaults assertions kept.

Full suite: 3975 passed / 13 skipped / 0 failed (was 3969 — +6 from
the new tests, no regressions).

## Follow-ups (separate issues worth filing)

- `keyboard.ts:85` `if (!isMobile)` skips key-event listeners. iPads
  with Magic Keyboard (now correctly identified) would stop receiving
  keys. Probably always-attach + let no-op-on-pure-touch sort itself
  out, but needs the iPad-with-keyboard test path to validate.
- Migrate to `navigator.userAgentData` (Client Hints) where available.
  Chromium-only today; Safari/Firefox lag. 20.x candidate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 1, 2026 10:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates melonJS platform detection to correctly classify modern iPads (iPadOS 13+) as iOS/mobile, while deprecating and removing dead-platform UA regexes from the isMobile aggregate to reduce noise and overhead.

Changes:

  • Detect iPadOS 13+ devices that present a desktop “Mac” UA via navigator.platform === "MacIntel" + maxTouchPoints > 1, and fold this into iOS / isMobile.
  • Deprecate legacy platform flags (wp, BlackBerry, Kindle, android2) and remove the dead-platform flags from the isMobile OR chain.
  • Add/adjust unit tests for the new isMobile wiring and the iPadOS 13+ detection contract; document the behavior in the changelog.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File Description
packages/melonjs/src/system/platform.ts Adds iPadOS 13+ detection logic, deprecates dead-platform flags, and simplifies isMobile aggregation.
packages/melonjs/tests/platform.spec.ts Updates isMobile expectation and adds contract tests for the iPadOS 13+ detection heuristic.
packages/melonjs/CHANGELOG.md Documents the iPadOS 13+ fix and deprecation/removal of dead-platform flags from isMobile.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/melonjs/src/system/platform.ts Outdated
Comment on lines +11 to +13
* wp `true` if the device is a Windows Phone platform (deprecated)
* BlackBerry`true` if the device is a BlackBerry platform (deprecated)
* Kindle`true` if the device is a Kindle platform (deprecated)
Comment on lines +45 to +47
/**
* @deprecated Android 2.x predates 2012. Will be removed in 20.x.
*/
Comment on lines +51 to +53
/**
* @deprecated Windows Phone was EOL'd by Microsoft in 2017. Will be removed in 20.x.
*/
Comment on lines +55 to +57
/**
* @deprecated BlackBerry stopped shipping BB10 devices in 2016. Will be removed in 20.x.
*/
Comment on lines +59 to +61
/**
* @deprecated Kindle has a negligible market share and behaves like Android. Will be removed in 20.x.
*/
Comment thread packages/melonjs/src/system/platform.ts Outdated
// `wp` / `BlackBerry` / `Kindle` — the underlying platforms are EOL
// and the regexes were burning cycles on every page load for
// hardware nobody ships.
export const isMobile = /Mobi/i.test(ua) || iOS || android || false;
Comment thread packages/melonjs/tests/platform.spec.ts Outdated
Comment on lines +142 to +145
it("does not flag a Mac touch-bar laptop (`maxTouchPoints === 1`)", () => {
// The check uses `> 1`, not `> 0`. A hypothetical single-point
// touch device should not trip it — multi-touch is iPad-class.
expect(isIPadOnMacUA({ platform: "MacIntel", maxTouchPoints: 1 })).toBe(
obiot and others added 2 commits June 1, 2026 18:36
`keyboard.ts:85` skipped attaching `keydown` / `keyup` listeners when
`isMobile === true`. The gate assumed "mobile = no physical keyboard"
— invalid in 2026 with iPads (now correctly identified post the
platform fix above) using Magic Keyboard, Bluetooth keyboards on
phones, Samsung DeX, ChromeOS tablet mode, etc.

Two empty listener slots cost essentially nothing on touch-only
devices; the handler's unbound-key path is a single map lookup
that returns undefined.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
945 lines / 53 exports / 56 JSDoc blocks of feature-detection helpers
and platform plumbing now ship as a `.ts` file with native type
signatures. JSDoc was already exhaustive so the conversion is mostly
mechanical — `@param {Type}` blocks become parameter annotations and
`@type {Type}` constants get TS-inferred.

Non-standard / legacy browser surfaces (`Document.mozFullScreenEnabled`,
`Navigator.standalone` / `browserLanguage` / `userLanguage`, iOS-only
`DeviceOrientationEvent.requestPermission`, deprecated
`Screen.lockOrientation`, `webkitAudioContext`) are typed via narrow
local intersection types declared at the top of the file.

Two small runtime improvements that fell out of the conversion:
- the cached `domRect` is now a real `DOMRect` (its `right`/`bottom`
  getters track `x + width` / `y + height` automatically, so the old
  explicit assignment of `domRect.right` was redundant);
- `onDeviceMotion` now guards against
  `e.accelerationIncludingGravity === null` rather than crashing.

Behavioural parity verified against the full 3975-test suite;
downstream call sites are unchanged thanks to bundler-resolution
rewriting `.js` imports to `.ts` source.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 1, 2026 10:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (4)

packages/melonjs/src/system/device.ts:44

  • domRect is now constructed via new DOMRect(...) at module load time. In Node/SSR (or any non-DOM environment), DOMRect may be undefined, causing an immediate ReferenceError just by importing device.ts. Please guard this creation and fall back to a lightweight object when DOMRect isn't available.
    packages/melonjs/src/system/device.ts:290
  • autoFocus was changed from an exported let to an exported const. This prevents consumers from disabling the autofocus behavior (me.device.autoFocus = false), which appears to be part of the public API per the JSDoc (@default true) and is used as a runtime flag (e.g. in pointerevent.ts).
    packages/melonjs/src/system/device.ts:246
  • The device.isMobile JSDoc list is now out of date: platform.isMobile no longer ORs BlackBerry / Windows Phone / Kindle, so the comment is misleading.
    packages/melonjs/src/system/device.ts:604
  • getElement never returns null (it falls back to document.body), but the JSDoc still says it can return null. This mismatch can confuse consumers and generated docs.

Comment thread packages/melonjs/src/system/platform.ts Outdated
Comment on lines +11 to +13
* wp `true` if the device is a Windows Phone platform (deprecated)
* BlackBerry`true` if the device is a BlackBerry platform (deprecated)
* Kindle`true` if the device is a Kindle platform (deprecated)
Comment on lines +45 to 62
/**
* @deprecated Android 2.x predates 2012. Will be removed in 20.x.
*/
export const android2 = /Android 2/i.test(ua);
export const linux = /Linux/i.test(ua);
export const chromeOS = /CrOS/.test(ua);
/**
* @deprecated Windows Phone was EOL'd by Microsoft in 2017. Will be removed in 20.x.
*/
export const wp = /Windows Phone/i.test(ua);
/**
* @deprecated BlackBerry stopped shipping BB10 devices in 2016. Will be removed in 20.x.
*/
export const BlackBerry = /BlackBerry/i.test(ua);
/**
* @deprecated Kindle has a negligible market share and behaves like Android. Will be removed in 20.x.
*/
export const Kindle = /Kindle|Silk.*Mobile Safari/i.test(ua);
Comment thread packages/melonjs/CHANGELOG.md Outdated
- **`Camera2d.updateTarget` smooth follow is now frame-rate independent.** Previously `pos.lerp(target, damping)` ran a parametric per-frame fraction — same `damping = 0.1` covered 10% of the gap per frame at 30Hz, 60Hz, 120Hz or 144Hz, so wall-clock convergence sped up linearly with the player's refresh rate. Now uses `pos.damp(target, lambda, dt)` with `lambda = -ln(1 - damping) * timer.maxfps`, which recovers the legacy per-frame fraction exactly at the configured target framerate AND keeps wall-clock convergence constant if the actual frame rate drifts. **No tuning change required** — existing `damping` values keep their feel at the engine's target framerate (default 60); high-refresh users finally get the same feel the dev tuned for. Dogfoods the new `math.damp` API on melonJS's most prominent older follow path.
- **`device.platform.isMobile` no longer ORs the dead-platform regexes** (#1467). `wp` / `BlackBerry` / `Kindle` regexes were burning cycles on every page load testing for hardware nobody ships (Windows Phone EOL 2017, BB10 EOL 2016, Kindle behaves like Android anyway). The remaining chain — `/Mobi/.test(ua) || iOS || android` — covers ~99.9% of mobile traffic in 2026 per MDN. The deprecated exports themselves still compute and return; only the `isMobile` aggregate stopped consulting them.
- **`initKeyboardEvent` no longer skips listener registration on `isMobile === true`** (#1467). The gate assumed "mobile = no physical keyboard" — invalid for iPads with Magic Keyboard (now correctly detected per the iPad fix above), Samsung DeX, ChromeOS tablet mode, Bluetooth-keyboard-on-phone, etc. Two empty listener slots cost nothing on touch-only devices; the unbound-key path is a single map lookup that returns undefined.
- **`system/device.js` converted to TypeScript** (#1467). 945 lines / 53 exports / 56 JSDoc blocks of feature-detection helpers and platform plumbing now ship as a `.ts` file with native type signatures. JSDoc was already exhaustive, so the conversion is mostly mechanical — `@param {Type}` blocks become parameter annotations and `@type {Type}` constants get TS-inferred. Non-standard / legacy browser surfaces (`Document.mozFullScreenEnabled`, `Navigator.standalone` / `browserLanguage` / `userLanguage`, iOS-only `DeviceOrientationEvent.requestPermission`, deprecated `Screen.lockOrientation`, `webkitAudioContext`) are typed via narrow local intersection types declared at the top of the file. Behavioural parity verified against the full 3975-test suite; downstream call sites (`pointerevent.ts`, `application.ts`, `resize.ts`, `header.ts`, etc.) are unchanged thanks to bundler-resolution rewriting `.js` imports to `.ts` source. Two small runtime improvements that fell out of the conversion: the cached `domRect` is now a real `DOMRect` (its `right`/`bottom` getters track `x + width` / `y + height` automatically, so the old explicit assignment of `domRect.right` was redundant), and `onDeviceMotion` now guards against `accelerationIncludingGravity === null` rather than crashing.
`prefer-const` flipped `export let autoFocus = true` → `const` during
the .js → .ts conversion lint pass because nothing in the module
reassigns it. The JSDoc still describes it as user-settable behaviour
("Specify whether to automatically bring the window to the front") —
`let` keeps the door open for an internal setter without forcing
another module-shape change.

Behaviourally moot today: ESM namespace-import bindings
(`device.autoFocus = false` via `import * as device`) are read-only
regardless of `let` / `const`, so external mutation never worked
either way. But intent matters.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot review batch from the platform.ts and device.ts review rounds:

- platform.ts header doc: add missing space before \`true\` for
  BlackBerry / Kindle entries (rendered as malformed markdown
  otherwise).
- platform.ts @deprecated: prepend `since 19.7.0 — ` to wp /
  BlackBerry / Kindle / android2 to match the codebase's existing
  convention (matches video.js, renderable.js, entity.js style).
- platform.ts isMobile: drop redundant `|| false` from the OR chain
  (every operand is already a boolean).
- tests/platform.spec.ts: rename "Mac touch-bar laptop" test —
  Touch Bar isn't a touchscreen and doesn't report maxTouchPoints.
  The test is about the `maxTouchPoints === 1` edge case directly.
- device.ts isMobile JSDoc: drop the dead-platform list (BlackBerry,
  Windows Phone, Kindle) — they're no longer in the isMobile OR
  chain per the upstream platform.ts change.
- device.ts getElement JSDoc: drop "or null if not existing" — the
  function falls back to `document.body` and never returns null.
- device.ts domRect cache: revert `new DOMRect(...)` → plain object
  literal so module load doesn't ReferenceError in Node / SSR
  environments where the DOMRect constructor isn't defined. The
  literal is cast to `DOMRect` at the return site.
- CHANGELOG: rephrase the `system/device` conversion entry to make
  the rename explicit ("renamed from device.js → device.ts" rather
  than referring readers to a path that no longer exists), and drop
  the (now-reverted) DOMRect claim.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 1, 2026 11:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

packages/melonjs/src/system/device.ts:136

  • touch and maxTouchPoints dereference globalThis.navigator.maxTouchPoints without guarding that navigator exists. In SSR/non-browser contexts (or environments with partial DOM polyfills), this can throw at module-load time and prevent importing device at all. Guard navigator consistently (similar to platform.ts) so both constants are safe to evaluate.
    packages/melonjs/src/system/device.ts:746
  • watchAccelerometer() requests permission via DeviceOrientationEvent.requestPermission, but the permission gate for devicemotion on iOS is DeviceMotionEvent.requestPermission(). Using the orientation constructor here can cause accelerometer setup to fail even though hasAccelerometer is true.
    packages/melonjs/src/system/device.ts:472
  • exitFullscreen assumes document.exitFullscreen() always exists and always returns a Promise (because .catch(...) is chained). In older fullscreen implementations you may only have vendor-prefixed exit APIs (or a void return), so this can throw even when hasFullscreenSupport is true.

Comment thread packages/melonjs/src/input/keyboard.ts Outdated
);
globalThis.addEventListener("keyup", keyUpEvent, false);
}
if (globalThis.addEventListener) {
Comment thread packages/melonjs/tests/platform.spec.ts Outdated
Comment on lines +105 to +110
// The module computes `iOS` at load time from `globalThis`, so
// these tests assert the LOGIC of the documented check by
// recreating it inline against stubbed navigator shapes. This
// is verification of the contract; the runtime-load value in
// real chromium is covered by the shape / desktop-defaults
// blocks above.
obiot and others added 2 commits June 2, 2026 07:36
…cation (#1467)

Fullscreen control finally has app-instance context. The canonical
path is now `app.requestFullscreen()` / `app.exitFullscreen()`,
defaulting to the app's `parentElement` (canvas + sibling HUD go
fullscreen together) and accepting an optional Element override.

The static `device.requestFullscreen()` / `device.exitFullscreen()`
helpers stay for backwards compat (still work through the deprecated
`getParent()` → `game.getParentElement()` global-game lookup), but
are now flagged `@deprecated since 19.7.0` pointing at the
Application methods.

Updates the two examples that wire `F` → toggle fullscreen
(platformer + platformer-matter) to use the new app-instance API.
Each `createGame.ts` calls `_app.requestFullscreen()` directly; each
HUD calls `game.requestFullscreen()` since the HUD code path doesn't
have an `_app` reference and `game` is already imported.

Implementation note: the new Application methods skip the
vendor-prefixed `webkitRequestFullscreen` / `mozRequestFullScreen`
probing the device wrappers do — every modern browser has unprefixed
`Element.requestFullscreen` since ~2018. Users on ancient browsers
that still need the prefix dance can fall back to the deprecated
`device.requestFullscreen()` path which preserves the legacy probing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…en probes (#1467)

Round-out of the move to app-instance fullscreen:

- **`Application#isFullscreen`** added so the trio sits together
  (`isFullscreen` / `requestFullscreen` / `exitFullscreen`). Defaults
  the documented example to `app.isFullscreen()` instead of mixing
  in `me.device.isFullscreen()`.
- **`device.isFullscreen` deprecated** alongside the other two
  fullscreen statics. Same `since 19.7.0 — use Application#…`
  pointer.
- The four example sites that still called `device.isFullscreen()`
  switch to `_app.isFullscreen()` / `game.isFullscreen()` so the
  fullscreen path is consistently app-instance in user-facing code.
- The new `Application#requestFullscreen` JSDoc names
  `parentElement` directly (with a backlink to
  {@link Application#getParentElement}) instead of the vaguer
  "canvas parent element" phrasing.

Tag-along cleanup of the deprecated device wrappers themselves: the
`prefixed("fullscreenEnabled", ...)` / `prefixed("fullscreenElement",
...)` / `prefixed("requestFullscreen", ...)` calls iterated 5 vendor
prefixes per probe via the `prefixed()` helper, with awkward
`as unknown as Record<string, unknown>` casts. Replaced with an
explicit four-variant OR chain (`fullscreenEnabled || webkit… ||
moz… || ms…`), the same pattern lib.dom.d.ts uses and what every
MDN recipe recommends in 2026. `DocumentLegacy` / `ElementLegacy`
gain the missing `webkit*` / `ms*` typings. `requestFullscreen`
also `.catch()`-es the Promise the modern (unprefixed) call
returns — the vendor-prefixed variants returned undefined so the
guard `if (result instanceof Promise)` cleanly covers both.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 1, 2026 23:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comment on lines +685 to +700
requestFullscreen(element?: Element): void {
if (device.hasFullscreenSupport && !this.isFullscreen()) {
const target = element ?? this.parentElement;
target.requestFullscreen?.().catch(console.error);
}
}

/**
* Exit fullscreen mode for this application.
* @category Application
*/
exitFullscreen(): void {
if (device.hasFullscreenSupport && this.isFullscreen()) {
globalThis.document.exitFullscreen().catch(console.error);
}
}
obiot and others added 2 commits June 2, 2026 07:47
…#1467)

`device.isFullscreen()` doesn't need Application context: the browser
tracks exactly one fullscreen element per document regardless of how
many Applications are running. Unlike `requestFullscreen` (needs to
know WHICH element) and `exitFullscreen` (paired with request),
`isFullscreen` is a stateless probe.

Drops:
- `@deprecated` JSDoc from `device.isFullscreen` + clarifies it's a
  document-state probe.
- the eslint-disable comments inside `device.requestFullscreen` /
  `device.exitFullscreen` that were silencing the now-non-deprecated
  internal call.
- the eslint-disable comment inside `Application#isFullscreen` for
  the same reason. The method now reads as a clean thin convenience
  wrapper.

`Application#isFullscreen` stays as a convenience so the trio reads
together (`isFullscreen` / `requestFullscreen` / `exitFullscreen`)
on the app instance, but its JSDoc now correctly identifies
`device.isFullscreen` as the canonical probe rather than implying
the device version is being phased out.

CHANGELOG updated to reflect the corrected scope of the deprecation
(request + exit only; isFullscreen stays).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round 3 (after ba60914):

- **keyboard.ts:84** — `if (globalThis.addEventListener)` →
  `typeof globalThis.addEventListener === "function"`. Matches the
  defensive style used in `device.initVisibilityEvents` and avoids
  calling a polyfilled non-function value.

- **platform.spec.ts iPadOS predicate drift** — the new tests were
  re-implementing `isIPadOnMacUA` locally inside the spec; if
  `platform.ts` accidentally changed the predicate (flipped `> 1`
  to `> 0`, dropped the `MacIntel` check, etc.) the tests would
  still pass. Fix: extract the predicate from `platform.ts` as an
  exported pure function (`isIPadOnMacUA(nav)`); the module's `iOS`
  computation now calls the exported function, and the spec asserts
  against the same function. No drift possible. (Tried the dynamic-
  import-with-stubbed-`globalThis.navigator` approach first, but
  Playwright browser mode makes `navigator` non-configurable enough
  that `vi.stubGlobal` doesn't propagate to dynamically-imported
  modules — extracting the predicate is the cleaner answer
  regardless.)

Round 4 (after 3b3fb5a):

- **application.ts:702 vendor-prefix gap** — `app.requestFullscreen`
  only probed `target.requestFullscreen?.()`. If
  `device.hasFullscreenSupport` was true via a vendor-prefixed flag
  (older WebKit / iOS), the unprefixed method was missing and the
  call silently no-op'd. The canonical app method now does the same
  four-variant probe the deprecated `device.requestFullscreen` does
  (`requestFullscreen || webkit… || moz… || ms…`), with a local
  `ElementWithLegacyFullscreen` type alias mirroring the
  `ElementLegacy` shape in `device.ts`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 2, 2026 00:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

packages/melonjs/src/system/device.ts:308

  • The eslint-disable prefer-const comment is misleading: it justifies let by claiming the variable is "reassignable via internal setters even though no setter exists today." There is no internal reassignment site (the only mutation today would be an external device.autoFocus = …, which ESM disallows on a module export binding regardless). If autoFocus is intended to be a public knob, it should be exposed via a setter / configuration object; otherwise this should be export const autoFocus = true; and the eslint-disable removed. As written, the comment encodes a contract (mutability) that the code does not actually provide.

Comment on lines +84 to 93
if (typeof globalThis.addEventListener === "function") {
globalThis.addEventListener(
"keydown",
(e) => {
keyDownEvent(e);
},
false,
);
globalThis.addEventListener("keyup", keyUpEvent, false);
}
Comment on lines +35 to +45
- **`device.platform.isMobile` no longer ORs the dead-platform regexes** (#1467). `wp` / `BlackBerry` / `Kindle` regexes were burning cycles on every page load testing for hardware nobody ships (Windows Phone EOL 2017, BB10 EOL 2016, Kindle behaves like Android anyway). The remaining chain — `/Mobi/.test(ua) || iOS || android` — covers ~99.9% of mobile traffic in 2026 per MDN. The deprecated exports themselves still compute and return; only the `isMobile` aggregate stopped consulting them.
- **`initKeyboardEvent` no longer skips listener registration on `isMobile === true`** (#1467). The gate assumed "mobile = no physical keyboard" — invalid for iPads with Magic Keyboard (now correctly detected per the iPad fix above), Samsung DeX, ChromeOS tablet mode, Bluetooth-keyboard-on-phone, etc. Two empty listener slots cost nothing on touch-only devices; the unbound-key path is a single map lookup that returns undefined.
- **`system/device` converted to TypeScript** (#1467, renamed from `device.js` → `device.ts`). 945 lines / 53 exports / 56 JSDoc blocks of feature-detection helpers and platform plumbing now ship as a `.ts` file with native type signatures. JSDoc was already exhaustive, so the conversion is mostly mechanical — `@param {Type}` blocks become parameter annotations and `@type {Type}` constants get TS-inferred. Non-standard / legacy browser surfaces (`Document.mozFullScreenEnabled`, `Navigator.standalone` / `browserLanguage` / `userLanguage`, iOS-only `DeviceOrientationEvent.requestPermission`, deprecated `Screen.lockOrientation`, `webkitAudioContext`) are typed via narrow local intersection types declared at the top of the file. Behavioural parity verified against the full 3975-test suite; downstream call sites (`pointerevent.ts`, `application.ts`, `resize.ts`, `header.ts`, etc.) are unchanged thanks to bundler-resolution rewriting `.js` imports to `.ts` source. One small correctness improvement fell out of the conversion: `onDeviceMotion` now guards against `accelerationIncludingGravity === null` rather than crashing.
- **`Application#requestFullscreen` / `Application#exitFullscreen`** — fullscreen control finally has app-instance context. `requestFullscreen` defaults to the app's `parentElement` (the container the canvas was appended into — `getParentElement()`), so the canvas plus any sibling HUD / overlay markup inside that container go fullscreen together; accepts an optional `Element` override. No deprecated `getParent()` / global-game lookup involved — the canonical fullscreen path now reaches the canvas through the Application it was created on. `Application#isFullscreen` is a thin convenience around the (still non-deprecated) `device.isFullscreen` so the trio reads together on the app instance; the underlying probe stays on `device` because there's exactly one fullscreen state per document regardless of how many Applications are running. The two examples that wire `F` → toggle fullscreen (platformer + platformer-matter) migrate to the new API.

### Deprecated
- **`device.requestFullscreen()` / `device.exitFullscreen()`** (#1467, since 19.7.0). Use `app.requestFullscreen()` / `app.exitFullscreen()` instead. The device wrappers still work for backwards compat through the 19.x line but rely on the deprecated global-game canvas lookup (`getParent()` → `game.getParentElement()`, deprecated since 18.3.0).
- **`device.platform.wp` / `BlackBerry` / `Kindle` / `android2`** (#1467). The underlying platforms are end-of-life (Windows Phone discontinued 2017, BlackBerry stopped BB10 in 2016, Android 2.x predates 2012, Kindle has negligible mobile-web share). Exports stay functional through the 19.x line for backwards compatibility — IDE warnings light up at consumer sites; removal scheduled for 20.x. For "is this a touch device?" use the existing `device.touch` flag (feature-detected via `navigator.maxTouchPoints` / pointer events).

### Fixed
- **`device.platform.iOS` / `device.platform.isMobile` now correctly identify iPads on iPadOS 13+** (#1467). Since Sept 2019, Safari on iPad has shipped the desktop Mac UA — no `iPad` token — so every modern iPad was falling through `isMobile` as desktop. The detection now layers a feature check on top of the UA regex: `navigator.platform === "MacIntel"` (Apple-frozen legacy string that persists on Apple Silicon Macs/iPads for backwards compat — NOT a CPU check) plus `navigator.maxTouchPoints > 1` (Macs don't have touchscreens; iPads do). Every internal consumer of `isMobile` (`keyboard.ts`, `application.ts`, `header.ts`) inherits the fix transparently.
obiot and others added 2 commits June 2, 2026 08:38
Uses the new `app.requestFullscreen()` / `app.isFullscreen()` /
`app.exitFullscreen()` trio so the showcase exercises the canonical
post-19.7 API. No HUD button — just the keyboard shortcut, matching
the rest of AfterBurner's minimal-chrome aesthetic.

Handler is registered + torn down inside `createGame`, so the
example's React mount/unmount cycle doesn't leak duplicate listeners
across remounts (which would stack up fullscreen toggles on every
key press).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit (b35cb5a) ended up only landing the
canvas-size tweak (1024×768 → 1024×576) — the actual F-key code I
intended to ship in that commit got clobbered by an IDE save before
`git add` ran. This commit adds the real change: a `KEYDOWN`
subscription in `createGame` that toggles `app.requestFullscreen()`
/ `app.exitFullscreen()` keyed off `app.isFullscreen()`, plus a
matching `event.off(...)` in the teardown function so the handler
doesn't leak duplicate registrations across React mount/unmount
cycles.

Exercises the canonical post-19.7 app-instance fullscreen trio
(consistent with the platformer + platformer-matter examples).
No HUD button — keeps AfterBurner's minimal-chrome aesthetic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 2, 2026 00:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

packages/melonjs/src/system/device.ts:308

  • The eslint-disable rationale "reassignable via internal setters even though no setter exists today" is contradictory — if there is no setter, prefer-const is correct, and the binding can't be reassigned from outside the module anyway (ESM exports are read-only on the consumer side). Either drop the let + eslint-disable and use const, or add the missing setter function. As written, me.device.autoFocus = false from user code throws in strict-mode ESM, so the apparent mutability is misleading API surface.

Comment on lines +37 to +38
- **`system/device` converted to TypeScript** (#1467, renamed from `device.js` → `device.ts`). 945 lines / 53 exports / 56 JSDoc blocks of feature-detection helpers and platform plumbing now ship as a `.ts` file with native type signatures. JSDoc was already exhaustive, so the conversion is mostly mechanical — `@param {Type}` blocks become parameter annotations and `@type {Type}` constants get TS-inferred. Non-standard / legacy browser surfaces (`Document.mozFullScreenEnabled`, `Navigator.standalone` / `browserLanguage` / `userLanguage`, iOS-only `DeviceOrientationEvent.requestPermission`, deprecated `Screen.lockOrientation`, `webkitAudioContext`) are typed via narrow local intersection types declared at the top of the file. Behavioural parity verified against the full 3975-test suite; downstream call sites (`pointerevent.ts`, `application.ts`, `resize.ts`, `header.ts`, etc.) are unchanged thanks to bundler-resolution rewriting `.js` imports to `.ts` source. One small correctness improvement fell out of the conversion: `onDeviceMotion` now guards against `accelerationIncludingGravity === null` rather than crashing.
- **`Application#requestFullscreen` / `Application#exitFullscreen`** — fullscreen control finally has app-instance context. `requestFullscreen` defaults to the app's `parentElement` (the container the canvas was appended into — `getParentElement()`), so the canvas plus any sibling HUD / overlay markup inside that container go fullscreen together; accepts an optional `Element` override. No deprecated `getParent()` / global-game lookup involved — the canonical fullscreen path now reaches the canvas through the Application it was created on. `Application#isFullscreen` is a thin convenience around the (still non-deprecated) `device.isFullscreen` so the trio reads together on the app instance; the underlying probe stays on `device` because there's exactly one fullscreen state per document regardless of how many Applications are running. The two examples that wire `F` → toggle fullscreen (platformer + platformer-matter) migrate to the new API.
let app: Application;
try {
app = new Application(1024, 768, {
app = new Application(1024, 576, {
Comment on lines +39 to +42
type NavigatorLike = { platform?: string; maxTouchPoints?: number };
export function isIPadOnMacUA(nav: NavigatorLike | undefined): boolean {
return nav?.platform === "MacIntel" && (nav?.maxTouchPoints ?? 0) > 1;
}
obiot and others added 2 commits June 2, 2026 09:43
Two related fixes:

1. **F-key handler moved inside the preload callback.** Registered
   at createGame's sync tail it was getting stripped on the first
   React StrictMode dev unmount (utils.tsx runs teardown on every
   useEffect cleanup) and never re-registered because the canvas-
   remount path doesn't re-invoke createGame. Registering inside
   the preload callback puts it alongside the rest of the game-
   running state, surviving StrictMode the same way `audio.playTrack`
   does. Matches the platformer / platformer-matter pattern.

2. **HUD positions read live viewport dimensions.** `HUD.ts`
   hardcoded `CANVAS_W=1024 / CANVAS_H=768`; after the recent
   1024×576 retune the GAME OVER overlay landed in the lower
   half (y=372 in a 576-tall world) and the bottom-edge credits
   ran off-canvas entirely (y=752). Now the constructor reads
   `app.viewport.width / .height` once and uses those everywhere
   (score, hi-score, lives, music + asset credits, GAME OVER
   + sub-line, DeathFlash bounds). The screen-projection ortho
   that drives `floating = true` renderables uses the same
   `viewport.width / .height` (see `Camera3d.screenProjection`),
   so the HUD positions track whatever Application size the
   example is configured with — no resize listener needed
   under `scale: "auto"` (default scaleMethod doesn't call
   `renderer.resize`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a "Powered by melonJS: https://melonjs.org" line at the
bottom-center of the AfterBurner HUD, between the existing
davidKBD music credit (bottom-left) and the Kenney art credit
(bottom-right). All three share the size 11 / muted #bbbbbb
tint so they read as one paired strip.

Tag-along touch-ups:
- Shorten the Kenney URL from `/assets/space-kit` to the bare
  `https://kenney.nl` root so the three lines balance on width.
- Move all three credits from `h - 16` → `h - 6` so they sit
  closer to the bottom edge of the viewport (more breathing
  room above for gameplay, less awkward gap below).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 2, 2026 09:54
Copilot flagged the public export as adding a forever-supported API
surface just for the spec's benefit. The function stays exported (the
spec asserts the SAME predicate the module evaluates at load time —
no drift), but the JSDoc now declares it as a test-seam with no
stability guarantee. TypeDoc with `--excludeInternal` hides it from
the generated docs; consumers reaching for it accept that the engine
can change / inline / rename it without a breaking-change bump.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

@obiot obiot changed the title fix(platform): catch iPadOS 13+ + deprecate dead-platform flags (#1467) feat: modernize platform/device detection + Application fullscreen API (#1467) Jun 2, 2026
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.

Modernize isMobile / platform detection — UA sniffing misses iPadOS 13+, includes dead platforms

2 participants