1653496 - Picture-in-Picture option not shown for VideoHTMLElement with VideoTrack from Twilio

Status: RESOLVED FIXED

(Toolkit :: Video/Audio Controls, defect, P3)

Description

[u666822]

Attachment: VideoHTMLElementScreenShot.png

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.149 Safari/537.36

Steps to reproduce:

I have implemented a VideoChat solution for a customer (2-5 participants share WebCamera, Microphone and Screen) that has a <video> tag in Angular (one per shown camera video). I attach a video track provided by twilio during runtime to the HTMLVideoElement:
<div class="{{ getCameraClassName() }}" [class.drawerOpen]="isDrawerOpen" [hidden]="isCamera1Hidden()">
<video #camera1 class="camera" [class.camerasOnTop]="isCamerasOnTop" controls muted="false"></video>
<span #cameraName1 class="name"></span>
</div>
in JavaScript:
const attachedMediaTrack = track.attach(mediaElement);

where track is a VideoTrack as documented in https://media.twiliocdn.com/sdk/js/video/releases/2.5.1/docs/VideoTrack.html
and mediaElement the HTMLVideoElement within the application.

I have set the muted flag to true and false without a change in behaviour. As the VideoTrack is a camera video from the user's webcam and sound is optionally streamed via another AudioTrack the VideoTrack has no audio, but I attached for testing reasons the AudioTrack to the VideoHTMLElement, which is not an option in production, but this has also changed anything in behaviour.

Actual results:

<video _ngcontent-anq-c95="" controls="" muted="false" class="camera" autoplay="" track-name="camera-Martin"></video>
with #shadow-root (closed):
<div class="videocontrols" xmlns="http://www.w3.org/1999/xhtml" role="none">
<link rel="stylesheet" href="chrome://global/skin/media/videocontrols.css">
<div id="controlsContainer" class="controlsContainer" role="none">
<div id="statusOverlay" class="statusOverlay stackItem" hidden="true">
<div id="statusIcon" class="statusIcon"></div>
<bdi class="statusLabel" id="errorAborted">Video loading stopped.</bdi>
<bdi class="statusLabel" id="errorNetwork">Video playback aborted due to a network error.</bdi>
<bdi class="statusLabel" id="errorDecode">Video can’t be played because the file is corrupt.</bdi>
<bdi class="statusLabel" id="errorSrcNotSupported">Video format or MIME type is not supported.</bdi>
<bdi class="statusLabel" id="errorNoSource">No video with supported format and MIME type found.</bdi>
<bdi class="statusLabel" id="errorGeneric">Video playback aborted due to an unknown error.</bdi>
</div>

      <div id="pictureInPictureOverlay" class="pictureInPictureOverlay stackItem" status="pictureInPicture" hidden="true">
        <div class="statusIcon" type="pictureInPicture"></div>
        <bdi class="statusLabel" id="pictureInPicture">This video is playing in Picture-in-Picture mode.</bdi>
      </div>

      <div id="controlsOverlay" class="controlsOverlay stackItem" role="none">
        <div class="controlsSpacerStack">
          <div id="controlsSpacer" class="controlsSpacer stackItem" role="none"></div>
          <div id="clickToPlay" class="clickToPlay" style="width: 204.9px; height: 204.9px;" hidden=""></div>
        </div>

        <button id="pictureInPictureToggleButton" class="pictureInPictureToggleButton" hidden="true">
          <div id="pictureInPictureToggleIcon" class="pictureInPictureToggleIcon"></div>
          <span class="pictureInPictureToggleLabel">Picture-in-Picture</span>
        </button>

        <div id="controlBar" class="controlBar" role="none" hidden="">
          <button id="playButton" class="button playButton" playlabel="Play" pauselabel="Pause" tabindex="-1" paused="true" aria-label="Play"></button>
          <div id="scrubberStack" class="scrubberStack progressContainer" role="none">
            <div class="progressBackgroundBar stackItem" role="none">
              <div class="progressStack" role="none">
                <progress id="bufferBar" class="bufferBar" value="0" max="3379" tabindex="-1"></progress>
                <progress id="progressBar" class="progressBar" value="100" max="100" tabindex="-1"></progress>
              </div>
            </div>
            <input type="range" id="scrubber" class="scrubber" tabindex="-1" max="3379">
          </div>
          <bdi id="positionLabel" class="positionLabel" role="presentation"></bdi>
          <bdi id="durationLabel" class="durationLabel" role="presentation"></bdi>
          <bdi id="positionDurationBox" class="positionDurationBox" aria-hidden="true">
            0:03<span class="duration" role="none" id="durationSpan"></span>
          </bdi>
          <div id="controlBarSpacer" class="controlBarSpacer" role="none" hidden=""></div>
          <button id="muteButton" class="button muteButton" mutelabel="Mute" unmutelabel="Unmute" tabindex="-1" aria-label="Mute"></button>
          <div id="volumeStack" class="volumeStack progressContainer" role="none">
            <input type="range" id="volumeControl" class="volumeControl" min="0" max="100" step="1" tabindex="-1" aria-label="Volume slider">
          </div>
          <button id="castingButton" class="button castingButton" aria-label="Cast to Screen" hidden=""></button>
          <button id="closedCaptionButton" class="button closedCaptionButton" hidden=""></button>
          <button id="fullscreenButton" class="button fullscreenButton" enterfullscreenlabel="Full Screen" exitfullscreenlabel="Exit Full Screen" aria-label="Full Screen"></button>
        </div>
        <div id="textTrackListContainer" class="textTrackListContainer" hidden="">
          <div id="textTrackList" class="textTrackList" offlabel="Off"><button class="textTrackItem" index="0" on="true">Off</button></div>
        </div>
      </div>
    </div>
  </div>

=> Firefox does not show the Picture-in-Picture button on the displayed video-element, although the same code works flawless in Chrome and Safari that both show the Picture-in-Picture button.

Expected results:

• The Picture-in-Picture option should be shown in the video's context menu resp. the Picture-in-Picture button on the right-hand side of the video
• In https://support.mozilla.org/en-US/kb/about-picture-picture-firefox you write "We only show it for videos that include an audio track, and that are also of sufficient size and play length." I don't understand why you close your Shadow DOM or why you implement restrictions like the one above that WebDeveloper have no chance to modify the DOM as requested by our users/customers. I got also requests to adapt the controls of the HTMLVideoElement, which Firefox does not allow me to do as the Shadow DOM is closed, while in Chrome it is not, such that I can do the adaptations as my customer requests me to do (e.g. don't display the play button). The same is true for Picture-In-Picture. I would really prefer that you open the DOM or that you provide means via CSS or HTML attributes that allows me to adapt the solution in a way the customers want me to do (=> display the Picture-in-Picture mode depending on a logic defined by the customer not by the browser).

Comment 1

[u666822]

btw the user agent I have used for testing has been:
"Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:77.0) Gecko/20100101 Firefox/77.0"
and not the Chrome that I have used to report the ticket.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

In bug 1592729, we disabled Picture-in-Picture for MediaStream videos because of the potential of a crash from our media stack. Unfortunately, we haven't had time to address the crash yet. Bug 1592539 tracks the work on fixing the crash.

Comment 3

[u666822]

Hi Mike,

Thanks for the feedback. I am looking forward to the fix of the bug!

Comment 4

[:Gijs (out for now; he/him)]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #2)

In bug 1592729, we disabled Picture-in-Picture for MediaStream videos because of the potential of a crash from our media stack. Unfortunately, we haven't had time to address the crash yet. Bug 1592539 tracks the work on fixing the crash.

That bug is fixed now, can we remove the condition at https://searchfox.org/mozilla-central/rev/62c443a7c801ba9672de34c2867ec1665a4bbe67/toolkit/actors/PictureInPictureChild.jsm#1127 and https://searchfox.org/mozilla-central/rev/62c443a7c801ba9672de34c2867ec1665a4bbe67/toolkit/content/widgets/videocontrols.js#175 to fix this bug?

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Ooooh - yes, I think so! We should at the very least try it out now.

Comment 6

[u666822]

Thank you for having a look into this. If you fix this, then it would be interesting to know from which version onward of Firefox the fix will be available.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Hey Reid - effectively, what we're going to want to do here is undo most of the changes from bug 1592729 (see the commits linked to from bug 1592729 comment 6). Specifically, we want to undo any of the changes that disabled the toggle. Any of the test refactorings can probably stay.

We'll also need to update browser_disabledForMediaStreamVideos.js. We should rename that test to browser_mediaStreamVideo.js, and then alter the test so that it ensures that Picture-in-Picture can be opened for MediaStream videos.

Comment 8

[Reid Shinabarker]

Attachment: Bug 1653496 - Picture-in-Picture option not shown for VideoHTMLElement with VideoTrack from Twilio. r?mconley,Gijs

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3722e6ee6ebd
Picture-in-Picture option not shown for VideoHTMLElement with VideoTrack from Twilio. r=mconley,Gijs

Comment 10

[Narcis Beleuzu [:NarcisB]]

Backed out for bc failures on browser_mediaStreamVideos.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/9e904ebcedefd8642bd999f8da120e0bf8291cd7
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=317664782&repo=autoland&lineNumber=5457

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1581160e62e6
Picture-in-Picture option not shown for VideoHTMLElement with VideoTrack from Twilio. r=mconley,Gijs

Comment 12

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/1581160e62e6