[MBL-19551][S/T/P] Immersive Video Player for Studio LTI Media Embeds #3772

suhaibabsi-inst · 2025-11-23T21:52:12Z

refs: MBL-19551
affects: Student, Teacher, Parent
builds: Student, Teacher, Parent
release note: Introduced immersive experience for video players

Test Plan

Create a page with a couple of videos embedded through Studio LTI.
In iOS app, on page appearance, "Open in Details View" should show up right below each video
Make sure to turn on flag "RCE Studio Embed improvements" on course level in order to see "Open in Details View" button, you should also see that button be removed when that flag is off.
Upon tapping on one of those buttons, a modal for the immersive media player must show up. See ticket's description for the visual requirements of that screen.

Checklist

refs: MBL-19480 affects: Student, Teacher, Parent builds: Student, Teacher, Parent release note: Introduced immersive experience for video player.

This reverts commit d459e0a.

…nvas-Uploads

refs: MBL-19551 affects: Student, Teacher, Parent builds: Student, Teacher, Parent release note: Introduced immersive experience for video players

claude · 2025-11-23T21:53:15Z

Claude Code Review

Updated: 2025-11-26

Build Status

⚠️ License Header Missing: CanvasLTIPostMessageHandler.js needs license header. Run yarn update-headers to fix.

Code Review

✅ APPROVED - No critical issues after refactoring:

HTML escaping properly implemented (InsertStudioOpenInDetailButtons.js:138-145)
SVG content safely escaped via CoreWebView.jsString() before injection
All URL construction wrapped in try-catch error handlers
Null/optional checks prevent crashes (searchParams.get() results validated)
window.detailLinkSpecs has defensive fallback (line 74)
File loading with proper guard statements and RemoteLogger error logging
DOM injection uses safe APIs (appendChild, textContent) - no innerHTML risks

✅ Approved (fix license header before merge)

inst-danger · 2025-11-23T22:02:56Z

	Warnings
⚠️	One or more files are below the minimum test coverage 50%
⚠️	The total test coverage is below the minimum 90%

Release Note:

Introduced immersive experience for video players

Affected Apps: Student, Teacher, Parent

Builds: Student, Teacher, Parent

MBL-19551

Coverage	New %	Master %	Delta
Canvas iOS	81.09%	81.06%	0.03%
Horizon/Horizon/Sources/Features/Dashboard/SkillsHighlightsWidget/Data/ProficiencyLevel.swift	0%	0%	0%
Horizon/Horizon/Sources/Features/Notebook/Common/View/HighlightWebView/HighlightWebFeature.swift	0%	0%	0%
Horizon/Horizon/Sources/Features/Notebook/Common/View/HighlightWebView/EnableZoom.swift	0%	0%	0%
Horizon/Horizon/Sources/Common/Domain/GetCoursesInteractor.swift	38.16%	38.16%	0%
Horizon/Horizon/Sources/Features/LearningObjects/Assignment/SubmissionComment/Data/CommentAttachment.swift	0%	0%	0%
Horizon/Horizon/Sources/Features/Account/Notifications/Domain/NotificationSettingsInteractor.swift	0%	0%	0%
Horizon/Horizon/Sources/Common/Data/HModuleStatus.swift	0%	0%	0%
Horizon/Horizon/Sources/Common/Utilities/Double+.swift	0%	0%	0%
Horizon/Horizon/Sources/Features/Session/SessionInteractor.swift	32%	33.33%	-1.33%
Horizon/Horizon/Sources/Features/Assist/AssistFlashCard/View/AssistFlashCardViewModel.swift	0%	0%	0%

Generated by 🚫 dangerJS against 8046ae2

inst-danger · 2025-11-23T22:24:29Z

Builds

Commit: Update InsertStudioOpenInDetailButtonsTests.swift (8046ae2)
Build Number: 898
Built At: Nov 27 22:55 CET (11/27 02:55 PM MST)

Student	Teacher	Parent

claude · 2025-11-26T07:51:00Z

Claude Code Review

Updated: 2025-11-26

Critical Issues Found

CSS Contains Unevaluated Swift Interpolation (InsertStudioOpenInDetailButtons.css:7): Literal \\(fontSize)px instead of actual CSS value. Browser receives invalid CSS, breaking button styling.
Fragile Query Parameter Detection (CoreWebViewStudioFeaturesInteractor.swift:197): Substring matching custom_arc_launch_type=immersive_view instead of parameter parsing. Could trigger on unintended query strings.
Partial Frame Replacement Silent Failures (StudioIFrameReplaceInteractor.swift:62): Changed from throwing error on first unmapped iframe to silently skipping and only throwing if ALL frames fail. Masks partial data issues.

❌ Issues found

… into feature/MBL-19551-Immersive-Player-Studio-Videos

claude · 2025-11-26T09:45:59Z

Claude Code Review

Updated: 2025-11-26

Critical Issues Found

CSS file contains unevaluated Swift interpolation (InsertStudioOpenInDetailButtons.css:7): Line contains literal \(fontSize)px instead of actual CSS value. This will render as invalid CSS and break button styling.
Query parameter detection fragile (CoreWebViewStudioFeaturesInteractor.swift:196): Using query.contains("custom_arc_launch_type=immersive_view") is substring matching, not parameter parsing. Could match unintended URLs.
Partial iframe replacement logic (StudioIFrameReplaceInteractor.swift:63): Changed from throwing error on first unmapped iframe to silently continuing. Now throws error only if ALL iframes fail to map, masking partial failures.

❌ Issues found

… into feature/MBL-19551-Immersive-Player-Studio-Videos

claude · 2025-11-26T22:05:22Z

Claude Code Review

✅ APPROVED

Summary:
No critical issues found. The PR implements immersive video player functionality for Studio LTI media embeds with proper error handling and code organization.

Key observations:

✅ StudioIFrameReplaceInteractor change is correct: changed from throwing when single iframe not found to only throwing if NO frames were replaced (allows partial replacements)
✅ JavaScript LTI handler properly prevents duplicate initialization with guard flag
✅ URL detection logic refactored correctly to handle both Canvas uploads and Studio embeds
✅ Proper string encoding/decoding for URL parameters (removingPercentEncoding)
✅ Feature injection pattern consistent with project conventions
✅ Test coverage added for new Studio embed immersive view URL detection

claude · 2025-11-26T22:10:32Z

Claude Code Review - Updated 2025-11-26

Critical Issues:

InsertStudioOpenInDetailButtons.css line 7: Invalid CSS syntax - contains unprocessed placeholder text
InsertStudioOpenInDetailButtons.js line 105: innerHTML assignment with SVG creates XSS vector
CanvasLTIPostMessageHandler.js: No cleanup for global message listener - causes accumulation on reapply
InsertStudioOpenInDetailButtons.swift line 109: Appends empty DOM elements when title is nil

Issues found

claude · 2025-11-27T10:53:46Z

Claude Code Review

Updated: 2025-11-27

Status: ⚠️ Issues Found

Critical Issues:

XSS Vulnerability (InsertStudioOpenInDetailButtons.js:91-92): SVG icon injected via innerHTML without sanitization. Even though Swift escapes via CoreWebView.jsString(), innerHTML parsing bypasses escaping—allows execution of embedded scripts.
Unvalidated URL Construction (InsertStudioOpenInDetailButtons.js:46): searchParams.get() returns optional value used directly in new URL() without null check—can throw exception.
Silent Failures: Missing .js/.css files cause feature to silently fail without logging (InsertStudioOpenInDetailButtons.swift).
License Header Missing: CanvasLTIPostMessageHandler.js fails CI validation per dangerJS.

Secondary:

Navigation link detection order changed without test coverage.
Query string check uses contains() vs. exact parameter match.

🚫 Issues found.

claude · 2025-11-27T13:59:23Z

Claude Code Review

Updated: 2025-11-27

✅ No critical issues found.

CanvasLTIPostMessageHandler.js properly validates origin and handles data safely
Navigation detection adds Studio embed check correctly (path suffix + query param validation)
CSS/JS refactored to external files appropriately
JS properly handles both Canvas uploads and Studio LTI embeds with separate link generation
Duplicate prevention using ios-injected flag is correct
Test coverage validates Canvas upload button injection

✅

rh12 · 2025-11-27T18:39:49Z

Core/Core/Common/CommonUI/CoreWebView/Model/Features/InsertStudioOpenInDetailButtons.css

+
+.open_details_button {
+    font-weight: 400;
+    font-size: \(fontSize)px;


I don't see where does this get its value.

Ooops!.
I copied that from the Canvas upload one, and totally forgot about this.

Fixed. We not longer need to calculate the size for that. Tested that on iOS 18, and iOS 26. With a fixed value of 14px would do the job for accessibility scaling.

iOS 26

iOS 18

rh12 · 2025-11-27T18:46:46Z

Core/Core/Common/CommonUI/CoreWebView/Model/CoreWebViewStudioFeaturesInteractor.swift

    /// later to set immersive video player title. This mainly useful when triggering the player
-    /// from a button that's internal to video-frame. (`Expand` button)
+    /// from a button that's internal to video-frame. (`Expand` button).
+    /// Only for Canvas uploads video players.


Please fix this sentence

rh12 · 2025-11-27T18:57:26Z

Student/Student/Assignments/AssignmentDetails/StudentAssignmentDetailsViewController.swift


    private var env: AppEnvironment = .defaultValue
-    private let webView = CoreWebView()
+    private let webView = CoreWebView(features: [.canvasLTIPostMessageHandler])


I see this is added for pages, quiz details, student assignment details.
Should it not need to be added for other places?
Parent and teacher assignment details?
Like syllabus or Inbox?
Not sure about discussions (the ones using the viewcontroller, and the new ones opening the web version)

Or should this be used by default in all webviews?
Or this is needed only when coming from UIKit?

cc @vargaat for input

@rh12

It should be used for all webViews meant to load content as HTML text as they lack the scripts to solve the issue of LTI frame resizing.
Theoretically speaking, it won't be necessary for WebView loading full URLs as those can load the needed Javascript scripts required for that and much more.

rh12 · 2025-11-27T20:55:58Z

Core/Core/Common/CommonUI/CoreWebView/Model/Features/InsertStudioOpenInDetailButtons.css

 .open_details_button {
    font-weight: 400;
-    font-size: \(fontSize)px;
+    font-size: 14px;


wasn't the whole point that this should scale? As in the previous PR

Yes, but turned out that fixing icon style solved the issue. Apparently the old style was blocking the scaling for the whole button.

… into feature/MBL-19551-Immersive-Player-Studio-Videos

claude · 2025-11-27T21:13:39Z

Claude Code Review - Updated 2025-11-27

Critical Issues Found

Security: Missing origin validation in LTI PostMessage handler - Message handler processes postMessage from ANY origin without validating event.origin. Allows malicious iframes to trigger frameResize or extract window size data. Requires whitelist validation.

Security: XSS via innerHTML in SVG icon injection - InsertStudioOpenInDetailButtons.js uses direct innerHTML for SVG without sanitization. If externalLinkData asset is compromised, enables arbitrary script execution.

Correctness: Changed error semantics in StudioIFrameReplaceInteractor - Changed from fail-fast to silent skipping. Only throws if ALL videos missing, not on partial replacements. Failures now silent, risking broken playback without warning.

Logic: Unsafe query string validation - Detection uses substring match instead of proper parameter parsing. Creates false positives if string appears anywhere in query value.

Correctness: Unsafe URL title decoding - Title from URL query parameter decoded without validation. Could accept null bytes or invalid Unicode sequences.

Memory: Multiple event listener accumulation - Script injections before flag is set create duplicate listeners. No cleanup on navigation.

❌ Issues found

claude · 2025-11-27T21:24:57Z

Claude Code Review

Updated: 2025-11-27

✅ APPROVED - No critical issues.

Key observations:

✅ StudioIFrameReplaceInteractor properly continues on unmapped iframe
✅ Query parameter validation acceptable for immersive_view detection
✅ CanvasLTIPostMessageHandler prevents duplicate initialization
✅ JavaScript URL construction wrapped in try-catch handlers
✅ HTML escaping properly applied via escapeHTML() and CoreWebView.jsString()
✅ Proper null/optional checks throughout
✅ CSS uses hardcoded font-size values
✅ Feature flag gating controls feature availability correctly

✅

suhaibabsi-inst · 2025-12-02T09:19:50Z

Will be handled as part of other work. (MBL-19574)

suhaibabsi-inst and others added 25 commits November 10, 2025 02:35

Fetching feature flag

6d96ddf

Add handler to present immersive view in a dedicated modal.

7210c7b

Fix merge conflicts

a49f33c

Full functional requirements

2aac275

Fix issues with flag loading

7a2ea28

Remove context from delegate

7c07632

refs: MBL-19480 affects: Student, Teacher, Parent builds: Student, Teacher, Parent release note: Introduced immersive experience for video player.

Organising things

92c5282

Fix title extraction for expand button, enhanced separation of concerns

2b4c697

Fix feature removal issues

7c48f36

Update InsertStudioOpenDetailViewButton.swift

1844e19

Button with icon

7b262c8

Unit tests

a5daf3b

Address review comments

6e4cd3b

Naming methods

304d865

Resolve memory leak

d459e0a

Revert "Resolve memory leak"

84abcf4

This reverts commit d459e0a.

Address code review comments

6a0059f

Merge branch 'master' into feature/MBL-19480-Immersive-VideoPlayer-Ca…

f7ea7b5

…nvas-Uploads

Update CoreWebViewTests.swift

b3bda43

Few fixes

243d795

Update InsertStudioOpenDetailViewButton.swift

5366c88

Address AI feedback

d2c3ae7

Merge branch 'master' into feature/MBL-19480-Immersive-VideoPlayer-Ca…

8b887fe

…nvas-Uploads

Working path for inserting links to studio embeds

242ad15

Studio embed detail link check

ec1f1a2

refs: MBL-19551 affects: Student, Teacher, Parent builds: Student, Teacher, Parent release note: Introduced immersive experience for video players

suhaibabsi-inst self-assigned this Nov 23, 2025

Address AI feedback

cca614d

suhaibabsi-inst added 2 commits November 26, 2025 12:43

Update CoreWebViewTests.swift

c6d99a2

Merge branch 'feature/MBL-19480-Immersive-VideoPlayer-Canvas-Uploads'…

c2027ba

… into feature/MBL-19551-Immersive-Player-Studio-Videos

suhaibabsi-inst added 4 commits November 27, 2025 00:38

Inject script to fix LTI iframes resizing

bb35c0f

Update InsertStudioOpenDetailViewButtonTests.swift

8f72e57

Update InsertStudioOpenDetailViewButtonTests.swift

15a0c49

Merge branch 'feature/MBL-19480-Immersive-VideoPlayer-Canvas-Uploads'…

19b10bc

… into feature/MBL-19551-Immersive-Player-Studio-Videos

suhaibabsi-inst added 2 commits November 27, 2025 01:08

Update CoreWebViewTests.swift

d16b2cb

Update CoreWebViewTests.swift

e18ad06

Update CanvasLTIPostMessageHandler.js

354e9c4

Exclude quiz embeds

6e44989

vargaat added 2 commits November 27, 2025 18:20

Fix font size.

cf6fedd

Add better name.

b045bb9

rh12 reviewed Nov 27, 2025

View reviewed changes

Update InsertStudioOpenInDetailButtons.css

e17cc04

rh12 reviewed Nov 27, 2025

View reviewed changes

suhaibabsi-inst added 2 commits November 28, 2025 00:07

Merge branch 'feature/MBL-19480-Immersive-VideoPlayer-Canvas-Uploads'…

9110b25

… into feature/MBL-19551-Immersive-Player-Studio-Videos

Update CoreWebViewStudioFeaturesInteractor.swift

643678b

Update InsertStudioOpenInDetailButtonsTests.swift

8046ae2

vargaat marked this pull request as draft November 28, 2025 11:05

Base automatically changed from feature/MBL-19480-Immersive-VideoPlayer-Canvas-Uploads to master November 28, 2025 14:46

suhaibabsi-inst closed this Dec 2, 2025

[MBL-19551][S/T/P] Immersive Video Player for Studio LTI Media Embeds #3772

[MBL-19551][S/T/P] Immersive Video Player for Studio LTI Media Embeds #3772

Uh oh!

Conversation

suhaibabsi-inst commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Plan

Checklist

Uh oh!

claude bot commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Build Status

Code Review

Uh oh!

inst-danger commented Nov 23, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Note:

Affected Apps: Student, Teacher, Parent

Builds: Student, Teacher, Parent

Uh oh!

inst-danger commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Builds

Uh oh!

claude bot commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Critical Issues Found

Uh oh!

claude bot commented Nov 26, 2025

Claude Code Review

Critical Issues Found

Uh oh!

claude bot commented Nov 26, 2025

Claude Code Review

Uh oh!

claude bot commented Nov 26, 2025

Claude Code Review - Updated 2025-11-26

Uh oh!

claude bot commented Nov 27, 2025

Claude Code Review

Status: ⚠️ Issues Found

Uh oh!

claude bot commented Nov 27, 2025

Claude Code Review

Uh oh!

rh12 Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

suhaibabsi-inst Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

suhaibabsi-inst Nov 27, 2025

Choose a reason for hiding this comment

iOS 26

iOS 18

Uh oh!

rh12 Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

rh12 Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

suhaibabsi-inst Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rh12 Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

suhaibabsi-inst Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

claude bot commented Nov 27, 2025

Claude Code Review - Updated 2025-11-27

Critical Issues Found

Uh oh!

claude bot commented Nov 27, 2025

Claude Code Review

suhaibabsi-inst commented Nov 23, 2025 •

edited

Loading

claude bot commented Nov 23, 2025 •

edited

Loading

inst-danger commented Nov 23, 2025 •

edited by atlassian bot

Loading

inst-danger commented Nov 23, 2025 •

edited

Loading

claude bot commented Nov 26, 2025 •

edited

Loading

suhaibabsi-inst Nov 27, 2025 •

edited

Loading

suhaibabsi-inst commented Dec 2, 2025 •

edited

Loading