[lutzhelm]

• Use image resolution from info response instead of Canvas - Image service may have a different resolution than the Canvas
• Use correct canonical image size URL parameter (full vs. max / w, vs. w,h) dependent on IIIF Image API version
• Offer manifest renderings for download, not only sequence renderings
• Ensure that the offered download links do not exceed maxArea / maxWidth / maxHeight
• Fix IIIF Presentation 3 not supported #63 along the way
• Offer any existing static image URL if no image service is present

The plugin also does not offer a download for every image service beyond the first; this will need to be adressed outside this PR. -> #91

[phil-plencner-hl]

Hi @lutzhelm,

At Harvard we recently discovered were are having the same issue with version 3 iiif manifests returning inaccurate download links. I tried some of our manifests against this branch and they work correctly. Here are the ones I tried:

• https://nrs.harvard.edu/URN-3:FHCL:103496523:MANIFEST:3
• https://nrs.lib.harvard.edu/URN-3:FHCL:29400954:MANIFEST:3
• https://nrs.lib.harvard.edu/URN-3:FHCL.LOEB:25853480:MANIFEST:3
• https://nrs.harvard.edu/URN-3:FHCL:106214677:MANIFEST:3

The version 2 iiiif manifests of the same objects also continue to work correctly on this branch:

• https://nrs.harvard.edu/URN-3:FHCL:103496523:MANIFEST:2
• https://nrs.lib.harvard.edu/URN-3:FHCL:29400954:MANIFEST:2
• https://nrs.lib.harvard.edu/URN-3:FHCL.LOEB:25853480:MANIFEST:2
• https://nrs.harvard.edu/URN-3:FHCL:106214677:MANIFEST:2

From my perspective this is working and can be merged. Is that something we can do, or should we bring it up at the next Mirador Community Call?

cc: @marlo-longley @cbeer

[phil-plencner-hl]

I'm also wondering if there is a way to port these changes to a Mirador 3 compatible version of the plugin? We have not upgraded to 4 yet, but would like the download bug fix if possible.

[phil-plencner-hl]

@lutzhelm @marlo-longley
I rebased this branch off of the main branch and re-ran my tests:

https://nrs.harvard.edu/URN-3:FHCL:103496523:MANIFEST:3
https://nrs.lib.harvard.edu/URN-3:FHCL:29400954:MANIFEST:3
https://nrs.lib.harvard.edu/URN-3:FHCL.LOEB:25853480:MANIFEST:3
https://nrs.harvard.edu/URN-3:FHCL:106214677:MANIFEST:3

The version 2 iiiif manifests of the same objects also continue to work correctly on this branch:

https://nrs.harvard.edu/URN-3:FHCL:103496523:MANIFEST:2
https://nrs.lib.harvard.edu/URN-3:FHCL:29400954:MANIFEST:2
https://nrs.lib.harvard.edu/URN-3:FHCL.LOEB:25853480:MANIFEST:2
https://nrs.harvard.edu/URN-3:FHCL:106214677:MANIFEST:2

This works great to me.

I'm setting this PR to "Ready for Review". Can this be merged? We can discuss it in the upcoming Mirador Community Call if you think that is best.

Thanks.

[marlo-longley]

Hi @phil-plencner-hl -- This PR branch currently has the same 8 commits applied twice, plus your merge-branch-into-itself commit. That's why the PR shows 31 commits instead of ~8. Please fix the rebase. Then I have a couple code review suggestions. Thanks!

[phil-plencner-hl]

Hi @phil-plencner-hl -- This PR branch currently has the same 8 commits applied twice, plus your merge-branch-into-itself commit. That's why the PR shows 31 commits instead of ~8. Please fix the rebase. Then I have a couple code review suggestions. Thanks!

@marlo-longley I believe I have fixed it now. Thanks!

[marlo-longley]

Left a few comments to change minor issues

[marlo-longley]

supportsRequest -- is this ever called? I'm not totally clear in its purpose. Could it fulfill the TODO required in line 143?

[phil-plencner-hl]

Thanks for the review — all four comments are addressed. Details below, since a couple of them turned out to be deeper than they looked.

calculateHeightForWidth returning width

Good catch, and it was a real bug rather than just a naming inconsistency. The only caller is thousandPixelWideImage(), which feeds the result straight into the download URL, so any image whose native width was exactly 1000px produced /full/1000,1000/0/default.jpg — a square request for a non-square image. Under Image API 3, where we emit both dimensions, that's a distorted download or a server error. Images of any other width were unaffected, which is why it went unnoticed.

Worth noting the branch was redundant anyway: the general formula on the next line already yields exactly imageInfo.height when width === imageInfo.width. I kept it as the minimal fix, but it could be deleted outright.

full vs max for v3

Fixed as suggested — size = version === 3 ? 'max' : 'full'. This is a compatibility fix rather than tidiness: Image API 3.0 removed full as a size keyword in favor of max, and max is required at every compliance level including level 0, whereas the old w,h form depends on the optional sizeByWh feature. That's exactly why it worked against the NLNZ fixture (which advertises sizeByWh) and would have failed against a level-0 v3 server. The full region is unchanged; only the size segment moved.

One behavior note: the previous version < 3 guard was false for undefined, so images with an unrecognized @context fell through to w,. They now get full, which is valid in Image API 1 and 2 and is the better guess for a real-but-unrecognized context.

isVersion3 dead code

Confirmed dead and removed — the declaration, the prop pass, and the now-unused getImageApiVersion import in MiradorDownloadDialog. CanvasDownloadLinks referenced it in neither its body nor its propTypes, so it was landing in an unread props bag. Removing the const collapsed the map callback to an implicit return, which is why that block's diff is a little wider than the removal itself.

supportsRequest and the level-0 TODO

Yes to both questions: it was never called anywhere, and it was clearly the intended body of that empty if. Its first branch is precisely the negation of the enclosing condition, so it replaced the block rather than filling it:

if (!supportsRequest(imageInfo, region, width, height)) {
  return undefined;
}

It needed one addition before it could be wired up. Every compliance level must serve the whole image at its native size (full in v1/v2, max in v3) whether or not that size appears in sizes, but the function only consulted sizes — so a level-0 server whose sizes array omits its native dimensions would have had its whole-image download suppressed. That's the exact request the max change above now generates, so the two interact:

if (region === 'full') {
  // every compliance level must serve the whole image at its native size
  // (`full` in Image API 1/2, `max` in 3), whether or not it is listed in `sizes`
  if (imageInfo.width === width && imageInfo.height === height) return true;
  return Boolean(imageInfo.sizes && imageInfo.sizes.some(
    (size) => size.width === width && size.height === height,
  ));
}

I also switched .filter(...).length > 0 to .some(...) and wrapped the result in Boolean(), matching the normalization used elsewhere in the file.

This surfaced a separate bug worth flagging

Turning on level-0 gating meant createCanonicalImageUrl returns undefined far more often — and no caller handled that. All four sites in CanvasDownloadLinks did `${createCanonicalImageUrl(...)}?download=true`, so undefined became the literal href undefined?download=true.

This was already reachable on main, independent of level-0 support: any server declaring a maxWidth/maxHeight/maxArea smaller than the full image hit requestExceedsMaximum, got undefined back, and rendered a broken download link. I verified this rather than assuming it — restoring the old caller makes the new test fail with href="undefined?download=true".

Fixed by centralizing construction in a downloadUrl(region, width, height) helper that appends ?download=true only when a URL exists, with all four builders routed through it. Four render paths now omit links instead of emitting dead hrefs: thousandPixelWideLink early-returns, linksForDefinedSizes filters, displayCurrentZoomLink checks the URL, and fullImageLink already had its ternary.

Tests

53 passing, up from 44, lint clean.

Six in iiifImageFunctions against new level-0 v2 and v3 fixtures: native size served as full/max despite being absent from sizes, an advertised size served, an unadvertised size → undefined, a non-full region → undefined, and no sizes at all → undefined. Four pinning the max/full keywords and both scaled forms against the real fixtures. Five for calculateHeightForWidth. Three in CanvasDownloadLinks covering level-0 link suppression and the maxWidth regression above.

One behavior change to be aware of when merging

Level-0 image servers now show fewer download options than before — correct per the spec, but user-visible.

[marlo-longley]

@phil-plencner-hl this looks good, thank you.
I have one more small cleanup request. These methods have been removed from the code and should be removed from PropTypes:

getCanonicalImageUri: PropTypes.func.isRequired
getHeight: PropTypes.func.isRequired
getWidth: PropTypes.func.isRequired

[phil-plencner-hl]

• https://nrs.harvard.edu/URN-3:FHCL:103496523:MANIFEST:3

@phil-plencner-hl this looks good, thank you. I have one more small cleanup request. These methods have been removed from the code and should be removed from PropTypes:

getCanonicalImageUri: PropTypes.func.isRequired getHeight: PropTypes.func.isRequired getWidth: PropTypes.func.isRequired

@marlo-longley These unused methods have been removed.

I failed to notice that the commit history is again an issue.
There are now 23 commits in this PR, that is not correct.

[marlo-longley]

I failed to notice that the commit history is again an issue.
There are now 23 commits in this PR, that is not correct. Please rebase this.

[phil-plencner-hl]

@marlo-longley Ok. it has been rebased.