Skip to content

html5: Adds event required by BBB App when sharing screen#23923

Merged
antobinary merged 1 commit intobigbluebutton:v3.0.x-releasefrom
gustavotrott:bbb-app-screenshare
Sep 19, 2025
Merged

html5: Adds event required by BBB App when sharing screen#23923
antobinary merged 1 commit intobigbluebutton:v3.0.x-releasefrom
gustavotrott:bbb-app-screenshare

Conversation

@gustavotrott
Copy link
Collaborator

Required for the new version of "BigBlueButton Tablet".
https://apps.apple.com/br/app/bigbluebutton-tablet/id1641156756

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 17, 2025

Walkthrough

  • Added a guarded mobile callback in screenshareHasEnded: after invoking the screen-share bridge STOP, it calls window.bbbMobileApp.onScreenshareStopRequest() if available.
  • Introduced a new global object window.bbbMobileApp in the mobile app service with method onScreenshareStopRequest() that calls callNativeMethod('stopScreenShare').
  • Removed prior interception/override of KurentoScreenShareBridge.stop; no longer wraps or forwards the original stop.
  • Retained existing global window.bbbMobileScreenShareBroadcastFinishedCallback behavior for broadcast finish.
  • No exported/public signatures changed besides the new global callback surface.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant UI as UI (Screenshare)
  participant Bridge as KurentoScreenShareBridge
  participant MobAPI as window.bbbMobileApp
  participant Native as Native Layer

  Note over UI,Native: New stop flow on screenshare end
  UI->>Bridge: STOP()
  Bridge-->>UI: stopped
  UI->>MobAPI: onScreenshareStopRequest()? (if defined)
  alt Mobile callback available
    MobAPI->>Native: callNativeMethod('stopScreenShare')
    Native-->>MobAPI: ack
  else No mobile callback
    Note over UI: No-op
  end
Loading
sequenceDiagram
  autonumber
  participant UI as UI (Stop Control)
  participant Global as window.bbbMobileScreenShareBroadcastFinishedCallback
  participant Native as Native Layer

  Note over UI,Native: Existing broadcast-finished hook (unchanged)
  UI->>Global: invoke()
  Global->>Native: handle broadcast finished
  Native-->>Global: ack
Loading
sequenceDiagram
  autonumber
  participant UI as UI (Legacy)
  participant Bridge as KurentoScreenShareBridge
  participant Native as Native Layer

  Note over UI,Native: Old vs New control path
  rect rgba(200,200,255,0.15)
  Note over UI,Native: Old (removed)
  UI->>Bridge: STOP()
  Bridge->>Native: (overridden) stopScreenShare
  end

  rect rgba(200,255,200,0.15)
  Note over UI,Native: New
  UI->>Bridge: STOP()
  UI->>window.bbbMobileApp: onScreenshareStopRequest()
  window.bbbMobileApp->>Native: stopScreenShare
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title concisely and accurately summarizes the main change: adding an event for the BigBlueButton mobile app to handle screen-sharing stop requests. It is specific to the mobile/screenshare behavior and does not include noisy file lists or vague wording. A reviewer scanning PR history will understand the primary intent from this title.
Description Check ✅ Passed The description states the change is required for the new BigBlueButton Tablet release and includes an app store link, which directly explains the motivation for adding the mobile screenshare event in this changeset. It is short but clearly related to the changes summarized in the diff and PR objectives. The description is on-topic and provides the necessary high-level context for reviewers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
bigbluebutton-html5/imports/ui/components/screenshare/service.js (1)

227-229: Prevent double-stop loops with native callback and handle the promise.

Calling native stop may trigger bbbMobileScreenShareBroadcastFinishedCallback, which clicks “stop” again. Add a one-shot guard and observe the promise.

Apply:

-  if (window.bbbMobileApp && window.bbbMobileApp.onScreenshareStopRequest) {
-    window.bbbMobileApp.onScreenshareStopRequest()
-  }
+  const maybeStop = window.bbbMobileApp?.onScreenshareStopRequest;
+  if (maybeStop && !window.bbbMobileApp.__stopRequestedByWeb) {
+    window.bbbMobileApp.__stopRequestedByWeb = true;
+    // Avoid unhandled rejection; clear guard afterward
+    Promise.resolve(maybeStop()).finally(() => {
+      delete window.bbbMobileApp.__stopRequestedByWeb;
+    });
+  }

Additionally (outside this hunk), consider guarding inside window.bbbMobileScreenShareBroadcastFinishedCallback:

if (window.bbbMobileApp?.__stopRequestedByWeb) {
  delete window.bbbMobileApp.__stopRequestedByWeb;
  return; // skip duplicate click if JS initiated stop
}
document.querySelector('[data-test="stopScreenShare"]')?.click();
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed612d3 and c5b6fd2.

📒 Files selected for processing (2)
  • bigbluebutton-html5/imports/ui/components/screenshare/service.js (1 hunks)
  • bigbluebutton-html5/imports/ui/services/mobile-app/index.js (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-package (bbb-html5)

@github-actions
Copy link

Automated tests Summary

All the CI tests have passed!

@bigbluebutton bigbluebutton deleted a comment from coderabbitai bot Sep 17, 2025
@ramonlsouza ramonlsouza added this to the Release 3.0 milestone Sep 18, 2025
@antobinary antobinary merged commit 7d3b0ec into bigbluebutton:v3.0.x-release Sep 19, 2025
31 of 32 checks passed
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.

4 participants