Exception thrown when trying trying to open the gallery too rapidly

vwong commented 3 years ago

Description

The following exception is thrown on some cases, when trying to open a gallery.

Uncaught TypeError: Cannot read properties of undefined (reading 'height')
    at Zoom.getDragAllowedAxises (main.a60c7c8283fe84ae530698590080312818752650c3c124b9a57963e0c79b014571dcda78117a56de34bcf2f5e6e1e8c44cb6ae710cd5103813a4cd2c7183ad51.js:3506)
    at HTMLDivElement.<anonymous> (main.a60c7c8283fe84ae530698590080312818752650c3c124b9a57963e0c79b014571dcda78117a56de34bcf2f5e6e1e8c44cb6ae710cd5103813a4cd2c7183ad51.js:4187)

Steps to reproduce

Visit https://www.lightgalleryjs.com/demos/thumbnails/ on mobile device (seems to happen mostly on Android)
Try to open the gallery by tapping twice on a thumbnail; use different areas, so that it doesn't get interpreted as a double click. An example would be clicking on the left and right of the same image rapidly

Expected result

No error is thrown

Actual result

Gallery opens anyway, but error monitoring logs (eg, Sentry.io) is filled with the error messages

Additional context

We've configured our error monitoring to ignore the error for now, but that's only a short term solution, as we'd rather know if users are affected by bugs in our 3rd party libraries.

sachinchoolur commented 3 years ago

Hey @vwong,

Thank you for reporting this issue. Which version of lightGallery do you use?

vwong commented 3 years ago

Sorry, forgot to mention, it's 2.2.0. Will try upgrading to 2.2.1.

sachinchoolur commented 3 years ago

Sure, a similar issue has been fixed in 2.2.1 - https://github.com/sachinchoolur/lightGallery/issues/1146

Please let me know if the issue persists in 2.2.1

vwong commented 3 years ago

Actually, I reproduced the error on your site too, as mentioned in the original post. Is that 2.2.1?

sachinchoolur commented 3 years ago

No, it is 2.3.0-beta-1 which does not have this fix. I need to cherry-pick the commit to 2.3.0-beta-1. I was planning to make the update today

vwong commented 3 years ago

Ok, I'll report back after deploying 2.2.1 and letting it run for a while.

vwong commented 3 years ago

Unfortunately, 2.2.1 did not fix the problem I'm having. Also, for further context, there might be 2 different errors, or at least 2 different places where the error might've occured.

My stack traces:

node_modules/lightgallery/plugins/zoom/lg-zoom.es5.js in getImageSize at line 182:16
node_modules/lightgallery/plugins/zoom/lg-zoom.es5.js in setZoomEssentials at line 277:32
node_modules/lightgallery/plugins/zoom/lg-zoom.es5.js in apply at line 122:23

And the other one is:

node_modules/lightgallery/plugins/zoom/lg-zoom.es5.js in getDragAllowedAxises at line 216:67
node_modules/lightgallery/plugins/zoom/lg-zoom.es5.js in apply at line 896:47

We've been unable to reproduce it consistently, so don't really know yet what the problem is, only that Sentry.io is reporting that it is happening for some users, mostly Android. Sentry reports that it is occurring at these URLs (and possibly much more):

sachinchoolur commented 3 years ago

Thanks for the info. I'll try reproducing it from my side.

sachinchoolur commented 3 years ago

Hey @vwong,

I tried many times but could not reproduce the issue from my side. Are you still getting error messages on Sentry?

vwong commented 3 years ago

Actually, I've just recently deployed the following patch (using patch-loader), which suppressed the error, explaining why you've been unable to reproduce it.

--- ../../../node_modules/lightgallery/plugins/zoom/lg-zoom.es5.js  1985-10-26 18:15:00.000000000 +1000
+++ lg-zoom.es5.js  2021-09-13 14:39:44.692027711 +1000
@@ -274,6 +274,9 @@
             .first()
             .get();
         this.rotateValue = this.getCurrentRotation(rotateEl);
+        if (!$image.get()) {
+            return;
+        }
         this.imageYSize = this.getImageSize($image.get(), this.rotateValue, 'y');
         this.imageXSize = this.getImageSize($image.get(), this.rotateValue, 'x');
         this.containerRect = this.core.outer.get().getBoundingClientRect();
@@ -635,7 +638,7 @@
                 if (!pinchStarted && Math.abs(distance) > 5) {
                     pinchStarted = true;
                 }
-                if (pinchStarted) {
+                if (pinchStarted && _this.containerRect) {
                     _this.scale = Math.max(1, initScale + -distance * 0.008);
                     _this.zoomImage(_this.scale);
                 }
@@ -882,7 +885,7 @@
         var _LGel;
         this.core.outer.on('mousedown.lg.zoom', function (e) {
             // Allow zoom only on image
-            if (!_this.isImageSlide()) {
+            if (!_this.isImageSlide() || !_this.containerRect) {
                 return;
             }
             var $item = _this.core.getSlideItem(_this.core.index);

It's a near-term hack until the underlying problem is resolved, of course. It seems some timing issue causes the containers to be undefined. When I tried to reproduce it, the slides still open, almost as if a subsequent event came by to clean it up and make it work again.

sachinchoolur commented 3 years ago

Thanks for the update.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

vishal-ranjan-codes commented 9 months ago

I am getting the same error when I zoom in a few times.

Here is the full error report -

Unhandled Runtime Error
TypeError: Cannot read properties of undefined (reading 'height')

Call Stack
Zoom.getDragAllowedAxises
node_modules\lightgallery\plugins\zoom\lg-zoom.es5.js (175:0)
HTMLDivElement.eval
node_modules\lightgallery\plugins\zoom\lg-zoom.es5.js (914:0)

skyflyer commented 7 months ago

Can you reopen this issue, as the error is still relevant? At least in version 2.7.2.

sachinchoolur / lightGallery