dimsemenov / PhotoSwipe

JavaScript image gallery for mobile and desktop, modular, framework independent
http://photoswipe.com
MIT License
24.11k stars 3.31k forks source link

Orientation change issue on iOS 10.3 devices #1315

Open bma25 opened 7 years ago

bma25 commented 7 years ago

First, thank you for creating Photoswipe.

Unfortunately i'm having an issue with Photoswipe on iOS when the orientation changes from portrait to landscape or vice-versa. I tested this on an iPhone 6 and an iPad Air 2, both running iOS 10.3, using both Chrome and Safari. The issue can be observed on the demo page: open any image, then rotate the device. The layout gets messed up, images are too small in landscape and too big in portrait.

img_0036

willbrich commented 7 years ago

I have a similar problem on the iPhone 7. When opening Photoswipe from Photoswipe's demo page with the phone in landscape mode, and the address bar visible then photswipe's caption bar is hidden or partially concealed. The problem turned up in ios 10.3 in Safari. When first hiding the address bar and thereafter opening Photoswipe the problem do not appear. Google Chrome works as before under ios 10.3.

bma25 commented 7 years ago

For me the problem exists in both chrome and safari, even if I'm opening Photoswipe after hiding the address bar

bma25 commented 7 years ago

Today I updated to iOS 10.3.1, the problem still persists. And btw on Android it works fine.

bma25 commented 7 years ago

I tried to fix this problem temporary by checking the viewport size every second like the script already does for iOS versions older than 8 (setting features.isOldIOSPhone to true for all iOS devices). But this only works as long as the page has not been scrolled...

kristianantov commented 7 years ago

Any solution found?

dimsemenov commented 7 years ago

So it seems if you read size of an element that depends on window width directly in window.resize event - the value would be incorrect.

I've made a reduced test case http://photoswipe.com/resize-bug-test.html as it seems multiple libraries having the same issue (for example similar lightbox in Google AMP project https://github.com/ampproject/amphtml/issues/8479 ). Steps to reproduce:

  1. Load the page on iPhone with iOS 10.3 Safari in portrait orientation. You'll see grey rectangle that fills 100% width and height of the window.
  2. Turn the device to landscape orientation. The page will print width of window and width of 100% width/height element, their widths should be the same.
  3. Turn the device back to portrait and the page will print new values. These widths will not match – the width of the window would be correct, but the width of the element would be the same as it was in landscape.

Gist of above HTML page in case I'll delete it from server https://gist.github.com/dimsemenov/cb9aa8357eae669d5ad2235b42f7fd10

Webkit bug https://bugs.webkit.org/show_bug.cgi?id=170595


If anyone has an idea on how to fix the problem - I'd appreciate the input.

I'll probably just delay the updateSize method by 500ms in orientationchange event, e.g.:

var _orientationChangeTimeout;
...
framework.bind(window, 'resize orientationchange scroll', self);
framework.unbind(window, 'resize orientationchange scroll', self);
...
_globalEventHandlers = {
       ...
            orientationchange: function() {
                clearTimeout(_orientationChangeTimeout);
                _orientationChangeTimeout = setTimeout(function() {
                    if(_viewportSize.x !== self.scrollWrap.clientWidth) {
                        self.updateSize();
                    }
                }, 500);    
            }

But it's quite disgusting and I'm not sure how reliable the solution is.

bma25 commented 7 years ago

Thank you for taking the time looking at this. Your solution is working for me, even if the 500ms delay isn't the most beautiful thing I've seen. Ultimately let's hope Apple will fix their bug so the original script will work again.

aghassemi commented 7 years ago

@dimsemenov Do you know if a Webkit bug has been raised with Apple for this?

dimsemenov commented 7 years ago

@aghassemi I did open a ticket via their "Bug Reporter" (https://bugreport.apple.com), but not sure if it's the correct place, as I haven't got any reaction yet.

aghassemi commented 7 years ago

@dimsemenov Would you please copy/paste your bug report to https://bugs.webkit.org/ as a P1 bug instead? Most likely it is a webkit bug and https://bugs.webkit.org/ is monitored.

alexander-akait commented 7 years ago

/cc @dimsemenov can you publish last version on npm?

Firsh commented 7 years ago

It's still not that utterly perfect, the dual-image problem is still visible for a little while. On actual and older devices it could take a few seconds instead of a split second. https://www.screencast.com/t/3jEZpJHKdmf

Do you think there will be a nicer solution for this, or let's wait for Apple's turn? I'm asking as I use this in a commercial WP plugin and people are waiting on me to release an update. But a beta tester customer expressed concerns that this is not perfect yet (though better than nothing).

alexander-akait commented 7 years ago

@Thefirsh maybe we should send issue to their bug tracker and refer here?

Larotu commented 7 years ago

I'm affraid Apple won't change this sooner or later. Similar lightboxes are showing the same problem. Is it possible to implement some workaround in JS? Or is there a method of locking a site to landscape orientation? I noticed 'lightgallery' has a slight orientation-problem too, but besides the delay, the repositioning of the image takes place faster.

rjgotten commented 7 years ago

If anyone has an idea on how to fix the problem - I'd appreciate the input.

@dimsemenov, don't hook the resize or orientationchange events on the window. Use percentage or viewport unit sizes on your container element and then detect size changes directly on that element. I'd suggest an approach like wnr/element-resize-detector which uses the nested element scroll detection method, possibly with some detection of and forward compatibility with the ResizeObserver API.

While this adds significant code footprint, it's the only way to fix things that's not based on continuous polling or unreliable (and probably far too long) timeout delays.

Firsh commented 7 years ago

Personally I had to add a second setTimeout inside orientationchange, timed at 1000ms. Within virtual macOS, the iPhone simulator sometimes showed it right after orientation change, but about half of the time the 500ms was too fast. I agree it's ugly to begin with and everybody can see the trick (it appears wrong for a -literally - split second). Especially since the clientWidth could be expensive (but a subsequent call to it is maybe ok).

rjgotten commented 7 years ago

@Thefirsh Even worse is that if you don't add some clever logic that sniffs and detects the bug or add sniffing based on user agent (yu----ck!), everyone else with an actually decent browser gets to suffer along with the Apple crowd.

(You'd think that for once Apple would be able to release a stable version of Safari that does not in one way or form cripple a critical component of the web platform...)