ProjectMirador / mirador

An open-source, web-based 'multi-up' viewer that supports zoom-pan-rotate functionality, ability to display/compare simple images, and images with annotations.
https://projectmirador.org
Apache License 2.0
558 stars 256 forks source link

Video source update fails to re-render Video Viewer on Chrome/Firefox #3486

Open DiegoPino opened 3 years ago

DiegoPino commented 3 years ago

Hi,

On Mirador 3.x We have observed the following issue when using Chrome/Firefox in conjunction with multiple Video.

With a IIIF Manifest that contains 2 or more consecutive Video IIIF Items, when pressing on the NavBar's Thumbnails/Structure Item links, the HTML correctly updates the <source src='something'> on a <video> element but the Video fails to update/re-render the Video Player.

Note: This is not an issue on Safari where the Media/Video integration seems to be "better?" or at least trigger a full Element Update on HTML change.

Issue seems to be related to how most browsers manage the Video DOM element. A simple update to the <source> inner html for an initialized Video Node will not trigger a change or a re-render.

Possible solution:

On Chrome I tested assigning the video DOM element to a Global variable, e.g temp1. Once assigned, setting via console temp1.src = "http://someurl/somenewvideo.mp4" triggered immediately an update on the Viewer. So, probably the solution is to not use https://github.com/ProjectMirador/mirador/blob/master/src/components/VideoViewer.js for updating the Video Element (only) but getting the NODE and setting the src property of the video DOM element directly.

In this example, to be safe : temp1.load() could also be called after the initial .src property update

Hope I explained the issue correctly, thanks!

DiegoPino commented 3 years ago

Hi, in the meantime I managed to get around this with some weird JS (not quick, had to learn this, not my language) . Please forgive my bad bad JS but we needed a workaround fast.

This of course does not cover (not even for us) all the use cases and I may keep doing some iterations but since I could not get any way of subscribing from Mirador to specific media/DOM events I went for this ugly piece

Given element_id the Element ID of the wrapper DOM element for a new Viewer this..

const mirador_window = document.getElementById(element_id);
                        var observer = new MutationObserver(function(mutations) {
                            let mirador_videos = document.querySelectorAll(".mirador-viewer video source");
                            if (mirador_videos.length) {
                                mutations.forEach(function (mutation) {
                                    if ((mutation.target.localName == "video") && (mutation.addedNodes.length > 0) && (typeof(mutation.target.lastChild.src) != "undefined" )) {
                                        mutation.target.src = mutation.target.lastChild.getAttribute('src');
                                    }
                                });
                            }
                        });
                        observer.observe(mirador_window, {
                            childList: true,
                            subtree: true,
                        });

I added a Mutation Observer to the whole thing. Because of how Mirador uses React to write the whole <video..><source../></video> block the Mutation is always of "type" Child-list with at least one Child added when I need to set the src property for the target (the video element).

because its filtering against any Video Element modification, this also works for new Workspaces that are added on the fly, etc, etc. Probably need to deal with Audio too (most likely the same need). The fact the observers is not recording attribute changes avoids an eternal loop when we internally update the .src property of the <video> element.

This is not good code (I know..) but it solves the urgency of the issue in firefox/chrome and works also on safari, at least for us. Only sharing to say "no rush" since I totally understand how the current component /react approach on outputting the whole video hierarchy may require a larger change to make it work natively.

Important Note: one of the issue I see with my "hack" here is that if for some reason a

Thanks

patdunlavey commented 2 years ago

We have this same problem for audio viewer. @DiegoPino 's workaround works for this as well (this can be more efficient/compact, I'm sure):

                        const mirador_window = document.getElementById(element_id);
                        var observer = new MutationObserver(function(mutations) {
                          let mirador_videos = document.querySelectorAll(".mirador-viewer video source");
                          if (mirador_videos.length) {
                            mutations.forEach(function (mutation) {
                              if ((mutation.target.localName == "video") && (mutation.addedNodes.length > 0) && (typeof(mutation.target.lastChild.src) != "undefined" )) {
                                mutation.target.src = mutation.target.lastChild.getAttribute('src');
                              }
                            });
                          }
                          let mirador_audios = document.querySelectorAll(".mirador-viewer audio source");
                          if (mirador_audios.length) {
                            mutations.forEach(function (mutation) {
                              if ((mutation.target.localName == "audio") && (mutation.addedNodes.length > 0) && (typeof(mutation.target.lastChild.src) != "undefined" )) {
                                mutation.target.src = mutation.target.lastChild.getAttribute('src');
                              }
                            });
                          }
                        });
                        observer.observe(mirador_window, {
                            childList: true,
                            subtree: true,
                        });