Fix: Inconsistent natural image dimensions in IE

[Minh-Ng]

@pramodsum
The data-scale attribute is set to infinity when an Image element has a naturalWidth and naturalHeight of 0. This circumvents the problem even though IE11 is supposed to support naturalWidth and naturalHeight. It might be that naturalWidth and naturalHeight are eventually set but the setting is not done on load. In any case, this seems to fix the issue (tested on IE11).

[pramodsum]

this.setOriginalImageSize(this.imageEl);

[Minh-Ng]

setOriginalImageSize is implemented as a static method right now. Is there any preference for avoiding static methods? I know that only one Viewer will actually be instantiated so there will not be much of a difference in our use case but I am advocating for the use of static methods to avoid the creation of monolithic utility files.

[tonyjin]

We don't have a policy one way or another, but I'd lean towards making this a normal private function in the base viewer until otherwise needed since that's how we've organized other similar helper functions. The moment this is required in a different viewer outside of the ImageBaseViewer inheritance chain, we can easily update this to a static method.

[Minh-Ng]

Updated to be non-static

[tonyjin]

{HTMLElement}

[tonyjin]

I think @JustinHoldstock mentioned at some point he prefers just disabling the eslint rule that warns against this when you mean to explicitly modify the passed in param. We also generally tend to keep the 'El' portion of a name to indicate an HTML Element.

Would look like:

/* eslint-disable no-param-reassign */
setOriginalImageSize(imageEl) {
    // implementation
}
/* eslint-enable no-param-reassign */

[tonyjin]

Is this the preferred way of checking whether natural size attributes are available? What happens for 0x0 or 0 by Y / X by 0 images?

[Minh-Ng]

For 0x0, the original width and height will be set to 1x1. It is not ideal but the original width/height are used to measure scale. Therefore if they are set to 0 we will have division by 0 that results in scale = Infinity.

naturalWidth and naturalHeight are supposed to be supported in IE 9+ but it doesn't appear to hold true. The reason for this PR is due to naturalWidth and naturalHeight being 0x0 on a non-zero image.

[tonyjin]

Do you need to do this? finishLoading() is triggered after the image element loads (see ImageViewer.js) and this should be before zoom() is called so the image should already be in its natural width/height.

This implementation makes sense for the generic use case, but is probably less efficient for the exact use case you're solving for here.

[Minh-Ng]

I had previously tested out just checking image.width and image.height in the onload function. It appears to have already been styled at that point, hence the need for this. I can go back through and see if there is a point in which the original loaded image can set its data attributes.

[tonyjin]

Instead of adding properties directly to an HTML element, it's better to add a data attribute, see: https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes

// Set
imageEl.dataset.originalWidth = 1;
imageEl.dataset.originalHeight = 1;

// Get
const originalWidth = imageEl.dataset.originalWidth;
const originalHeight = imageEl.dataset.originalHeight;