PR #7893 (“enable videoOrientExt for tablets”) aims to fix rotated WebRTC screenshots on iPads but is unreliable: modern iPadOS 13+ Safari reports a macOS desktop user-agent, so server-side ua-parser-js cannot distinguish an iPad from a Mac, and the new isTablet() check evaluates to false for exactly the devices the fix targets.

Verified Finding — the fix does not work for modern iPads

Modern iPad (iPadOS 13+, Safari) sends a Macintosh; Intel Mac OS X ... user-agent. ua-parser-js v1 parsing that string yields device.type === undefined and browser.name === 'Safari'. The new expression isTablet() || !(isSafari() || isMobile()) then evaluates to false || !(true) = false, so videoOrientExt stays disabled — the bug is not fixed. The PR only helps when the device’s UA actually parses as device.type === 'tablet' (older iPad UA, or a WebView wrapper with a custom UA).

Ticket Context

FieldValue
TicketFKITDEV-8533
PRTechTeamer/vuer_oss #7893
Head → Basefeature/FKITDEV-8533customization/generali-atvilagitas
Reviewer stateCHANGES_REQUESTED
CustomerGenerali
Affected flowCustomer identification self-service (room 11216)

Reported Symptom

Generali reported rotated WebRTC screenshots and recordings in the customer identification self-service flow. The commit message frames the root cause as: iPad Pro fails to pre-rotate video when switching cameras. The intended fix is to make Janus negotiate the video orientation RTP extension (videoOrientExt) for tablets, so the receiver corrects orientation.

How videoOrientExt Is Gated

videoOrientExt (the WebRTC urn:3gpp:video-orientation RTP header extension) is currently disabled for Safari and mobile by this expression at 4 call sites:

!((customer.isSafari()) || (customer.isMobile()))

Call sites:

  • server/cv/VuerCVListenerSession.js
  • server/socket/events/videochat.js
  • server/transport/session/RoomTransportSession.js
  • server/transport/session/SelfServiceTransportSession.js

What PR #7893 Changes

PR #7893 adds a new method Customer.prototype.isTablet() (in server/db/model/customer.js) and ORs it into each of the 4 sites:

customer.isTablet() || !((customer.isSafari()) || (customer.isMobile()))

isTablet() uses ua-parser-js and returns true when device.type === 'tablet'.

Why the Fix Is Logically Hollow

For Agents

isTablet() is a strict logical subset of isMobile(). The existing isMobile() already returns true when device.type is 'mobile' OR 'tablet'. Therefore isTablet() can never be true unless isMobile() is also true.

This makes the new expression a clean dichotomy with no useful middle ground:

Device detected as tablet?isTablet()isMobile()New expr isTablet() || !(isSafari() || isMobile())Effect
Yestruetrue (already)truevideoOrientExt enabled — but isMobile() already knew it was a tablet, so isTablet() is redundant
Nofalsedepends!(isSafari() || isMobile())unchanged from beforeFix changes nothing

So either:

  • the device parses as a tablet → isMobile() was already true, and the OR’d isTablet() term is redundant; or
  • the device does not parse as a tablet → isTablet() is false, and the expression collapses back to the original gate.

The PR adds a method that, by construction, cannot change behaviour beyond what isMobile() already determined.

The Real Problem: iPadOS Spoofs a macOS User-Agent

iPadOS 13+ Safari sends a desktop Mac UA

Since iPadOS 13, Safari on iPad requests sites with a desktop-class user-agent string: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/.... There is no iPad token in the string. Apple did this deliberately so iPads receive desktop site layouts.

Consequences for server-side detection:

  • ua-parser-js parsing a Macintosh; Intel Mac OS X string returns device.type === undefined — it cannot tell an iPad apart from a MacBook by UA string alone.
  • browser.name comes back as 'Safari'.
  • customer.userAgent is populated server-side via setConnectionInfo(ip, locale, userAgent)only the raw UA string reaches the server.
  • Safari does not send Sec-CH-UA client-hint headers, so there is no secondary signal the server could fall back to.

For a modern iPad in the Generali flow:

isTablet()  = false   (device.type undefined)
isMobile()  = false   (device.type undefined)
isSafari()  = true    (browser.name === 'Safari')

new expr = isTablet() || !(isSafari() || isMobile())
         = false      || !(true       || false)
         = false      || !(true)
         = false      || false
         = false

videoOrientExt stays disabled → rotated screenshots persist → bug not fixed.

The PR only works when the device’s UA genuinely yields device.type === 'tablet':

  • an older iPad UA string (pre-iPadOS 13, or “Request Mobile Website” mode), or
  • a WebView wrapper / native SDK that sets a custom UA containing a tablet token.

ua-parser-js Version Note

For Agents

The repo uses ua-parser-js ^1.0.39 (v1). A reviewer linked the withFeatureCheck API — that API is documented on the ua-parser-js v2 docs site and is not available in the v1 the repo actually depends on. Any suggestion built on withFeatureCheck does not apply without a major-version bump.

Correct Fix Direction

Server-side UA parsing alone cannot detect a modern iPad. The reliable signal lives on the client:

const isIPad = navigator.maxTouchPoints > 1 && /Macintosh/.test(navigator.userAgent);

navigator.maxTouchPoints > 1 is true on iPads (touch screen) and false on Macs (Mac trackpads do not report touch points), so combined with the Macintosh UA token it positively identifies an iPad masquerading as a Mac.

The fix should:

  1. Run this detection client-side (in vuer_css / the customer browser).
  2. Pass the result to the server (an explicit flag, not derived from the UA).
  3. Have the server gate videoOrientExt on that explicit flag rather than re-parsing the UA.

Reviewer State

PR #7893 is CHANGES_REQUESTED.

ReviewerPositionAssessment
Pocok256Flagged the isTablet()isMobile() redundancy and the unreliable UA-based detectionCorrect — matches this analysis
chrismakaaySuggested narrowing isMobile() to match 'mobile' onlyDoes not address the bug; a refactor that risks breaking the other ~7 isMobile() callers

Do not narrow isMobile()

isMobile() matching 'mobile' OR 'tablet' is relied on by ~7 other call sites. Narrowing it to 'mobile'-only is a behavioural change with blast radius far beyond this ticket, and it still does not solve the rotated-screenshot bug (a modern iPad parses as neither 'mobile' nor 'tablet' anyway).

Takeaways for Future Work

Device detection in vuer_oss

  • Never trust server-side UA parsing to detect an iPad. iPadOS 13+ Safari is indistinguishable from macOS Safari by UA string. Use client-side navigator.maxTouchPoints and pass an explicit flag.
  • customer.isTablet() (if merged as-is) is a logical subset of customer.isMobile() — it adds no detection power. Treat it as a no-op for modern iPads.
  • customer.userAgent is the only client signal the server has for this — no client hints, no Sec-CH-UA.
  • The repo is on ua-parser-js v1; v2-only APIs (withFeatureCheck) are not available.
  • For any WebRTC video-orientation work: videoOrientExt is gated at 4 sites — VuerCVListenerSession.js, socket/events/videochat.js, RoomTransportSession.js, SelfServiceTransportSession.js. Keep them in sync.