Repository navigation
fix(util): use own document in toggleFullScreen() - #3552
Merged
kumilingus merged 3 commits intoOct 16, 2026
Merged
Conversation
toggleFullScreen() read window.top.document, which a cross-origin iframe may not access, so every call there threw a SecurityError. It now works on the element's ownerDocument, or the current document without an element, whose body becomes the default. On a top-level page nothing changes. In a same-origin iframe the default element is now the iframe's body instead of the top-level page's, and whether to enter or leave fullscreen follows the iframe's own fullscreen element.
toggleFullScreen() without an element must leave fullscreen through the current page when that page reports an element in fullscreen, as the second call of a toggle does. No test covered that branch; under Karma it fails on code that reads window.top.document. The helper that lists the fullscreen names is renamed, and its comment says which list it mirrors.
An embedded page in fullscreen puts its iframe in fullscreen on the embedding page too, so the exit test without an element now stubs the embedding page in fullscreen. Code that reads window.top.document then exits there, which the test catches; the old stub only caught a request no browser makes.
MartinKanera
force-pushed
the
fix/toggle-fullscreen-cross-origin
branch
from
October 15, 2026 22:01
b80cac4 to
1d8b768
Compare
MartinKanera
marked this pull request as ready for review
October 15, 2026 22:01
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix preserves fallback behavior, includes regression coverage, and has no identified blocking issues.
No open issues 🎉
What changed in this PR
Fixes util.toggleFullScreen() for cross-origin iframe applications by using the element’s own document rather than the top-level document.
Changes:
- Uses the current document’s body when no element is supplied.
- Adds four regression tests for document-specific fullscreen behavior.
- Includes a patch changeset for
@joint/core.
| File | Description |
|---|---|
| packages/joint-core/test/jointjs/core/util.js | Tests fullscreen entry and exit across document contexts. |
| packages/joint-core/src/util/util.mjs | Uses the appropriate document and updates API documentation. |
| .changeset/toggle-fullscreen-own-document.md | Records the fullscreen fix for release. |
Review effort: Balanced
kumilingus
approved these changes
Oct 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
util.toggleFullScreen()readwindow.top.document, which a cross-origin iframe is not allowed to read. So an app embedded cross-origin got aSecurityErroron every call, even when its embedding page allows fullscreen. It now works on the element's own page (el.ownerDocument), or on the current page when no element is given, whosebodybecomes the default.Changes
util.toggleFullScreen()enters or leaves fullscreen based on the element's own page instead ofwindow.top.document, and without an element it uses the current page'sbodyinstead of the top-level page's. The order in which it tries the prefixed and standard names is unchanged.bodyenters fullscreen, and the current page exits its own fullscreen without touching the embedding page.@joint/corepatch changeset.Verification
yarn workspace @joint/core lint: passyarn workspace @joint/core test: pass (mocha 4/4, Karma joint 2164/2164, geometry 350/350, vectorizer 453/453 with coverage thresholds, qunit,ts:test,ts:test-exports); the 4 new tests fail without the fix.yarn changeset status --since=upstream/master:@joint/corepatchtoggleFullScreen()with and without an element.allow="fullscreen": a click enters fullscreen and a second click leaves it, with no errors.allow: noSecurityError, and nothing enters fullscreen. Chrome stays silent; Firefox rejects with an uncaughtTypeError: Fullscreen request denied, as it already does for a refused request on a top-level page.