Mirador fails to load Manifests with layers

[regisrob]

Testing the mui5-react-18 branch, I notice that Manifests with layers make Mirador crash.

It fails with all the Manifests in the layers.html demo, for instance this one: https://dvp.prtd.app/hamilton/manifest.json

This Manifest in the default index.html demo also fails with same error. To reproduce the bug, open the Biblissima Manifest Manuscrit reconstitué : Châteauroux, Bibliothèque municipale, ms. 5 (Grandes Chroniques de France), open the Index panel, click on the first entry in the ToC ("f. 34r...").

In both cases it raises this JS error:

Uncaught (in promise) Error: Index bigger than number of layers.
    setItemIndex webpack-internal:///./node_modules/openseadragon/build/openseadragon/openseadragon.js:8452
    refreshTileProperties webpack-internal:///./src/components/OpenSeadragonViewer.js:43
    refreshTileProperties webpack-internal:///./src/components/OpenSeadragonViewer.js:43
    addAllImageSources webpack-internal:///./src/components/OpenSeadragonViewer.js:41
world.js:153:18

[lutzhelm]

I believe I stumbled upon this before but I failed to create an issue, so thank you for doing that! As far as I remember, this is not an issue that was introduced with the MUI 5 or React 18 changes.

I can't reproduce the issue every time consistently; it seems to be caused by race conditions. In certain circumstances, the _items in the OSD viewer object do only contain one item (or at least not one item for each Layer), while the CanvasWorld uses all available image resources of the canvas to calculate the new index:

mirador/src/lib/CanvasWorld.js

Line 166 in 593e81b

total: miradorCanvas.imageResources.length,

mirador/src/lib/CanvasWorld.js

Line 186 in 593e81b

return layer.total - layer.index - 1;

https://github.com/openseadragon/openseadragon/blob/59645e3a0dbc321be0f503df4e6fb4b156c3fbba/src/world.js#L152-L154

[lutzhelm]

I think I have found two possible sources for race conditions. The first one has a lot more impact because it depends on network connections.

Image info responses that are added to viewer vs. number of image resources on the canvas according to the manifest

The props that are provided to the OpenSeadragonViewer component do only contain image infoResponses that have already been successfully fetched.

mirador/src/containers/OpenSeadragonViewer.js

Lines 34 to 37 in 593e81b

	infoResponses: imageServiceIds.map(id => infoResponses[id])
	.filter(infoResponse => (infoResponse !== undefined
	&& infoResponse.isFetching === false
	&& infoResponse.error === undefined)),

On the other hand, the index that is calculated for the OpenSedragonViewer in CanvasWorld.getLayerMetadata()

mirador/src/lib/CanvasWorld.js

Lines 157 to 158 in 593e81b

	const resourceIndex = miradorCanvas.imageResources
	.findIndex(r => r.id === contentResource.id);

uses the images resources as they are found in the manifest:

mirador/src/lib/MiradorCanvas.js

Lines 68 to 82 in 593e81b

	get imageResources() {
	const resources = flattenDeep([
	this.canvas.getImages().map(i => i.getResource()),
	this.canvas.getContent().map(i => i.getBody()),
	]);
	return flatten(resources.map((resource) => {
	switch (resource.getProperty('type')) {
	case 'oa:Choice':
	return new Canvas({ images: flatten([resource.getProperty('default'), resource.getProperty('item')]).map(r => ({ resource: r })) }, this.canvas.options).getImages().map(i => i.getResource());
	default:
	return resource;
	}
	}));
	}

I'm not sure what a quick fix would look like.

setTimeout() in OSD code

I believe there is another possible source for race conditions in how OSD itself handles the addition of tilesources to the world object. While the viewer component waits for all Promises before it calls the function where the error manifests,

mirador/src/components/OpenSeadragonViewer.js

Lines 188 to 191 in 593e81b

	return Promise.allSettled([
	...infoResponses.map(infoResponse => this.addTileSource(infoResponse)),
	...nonTiledImages.map(image => this.addNonTiledImage(image)),
	]).then(() => {

OSD's world.addItem() is called via setTimeout():

https://github.com/openseadragon/openseadragon/blob/59645e3a0dbc321be0f503df4e6fb4b156c3fbba/src/viewer.js#L2605

In this setTimeout(), successCallback() is either called directly or via waitUntilReady(); successCallback for getTileSourceImplementation() in viewer.addTiledImage() calls processReadyItems() where world.addItem() is called.

https://github.com/openseadragon/openseadragon/blob/59645e3a0dbc321be0f503df4e6fb4b156c3fbba/src/viewer.js#L1719-L1725

I'm not sure if the resolution of all promises is sufficient or if there might still be asynchronous code running to add those items.

Both issues seem neither easy to debug nor to resolve.

[enriquediaz]

Hi @regisrob 👋 based on the findings @lutzhelm has shared above, it appears that the issue you identified has been present for some time, and deeply-seated at that, from before the mui5-react-18 branch was created.

I'm going to update the title of this issue, "Mirador fails to load Manifests with layers on mui5-react-18 branch," to remove the name of the branch specifically since it is not related to it, but also keep it open in the hope that progress can be made.

Apart from this, did you find any other bugs in the mui-react-18 branch?

[regisrob]

👋 @enriquediaz thanks for that, and thanks @lutzhelm for the investigation on this bug (which I never noticed before with previous versions of Mirador 3). This one looks a bit tricky...

I did not find any other bugs in the mui-react-18 branch so far.

[lutzhelm]

openseadragon/openseadragon#2457 seems to be related, but I'd assume that the cause is rather in Mirador than in OpenSeadragon.

[BeebBenjamin]

That's my issue. It is caused by Mirador. I have had this error in the console for several years in Mirador 3. I previously thought it was an OSD issue (mistakenly).