wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
663 stars 210 forks source link

Delay JS - Links needs to be clicked twice on iOS / Safari #3142

Open arunbasillal opened 3 years ago

arunbasillal commented 3 years ago

Before submitting an issue please check that you’ve completed the following steps:

Describe the bug On Safari browser in iOS (and probably in others as per one customer), when delay JS is enabled links have to be clicked twice for them to work. This is even when:

To Reproduce Steps to reproduce the behavior:

Note that if you click on a link which is in the viewport, i.e. without scrolling, it will open immediately.

Expected behavior Links should load as normal and shouldn't need two clicks.

Screenshots Screencast: https://youtu.be/D1Pp45wLMhE (Thanks to @vmanthos for this and most of this report)

Additional context Please see this note from Vasilis for some more insights - https://secure.helpscout.net/conversation/1283702266/195256/#thread-3691038967

Potentially related tickets:

Backlog Grooming (for WP Media dev team use only)

socialpreneur commented 3 years ago

I had a client reporting this with Flying Scripts plugin, too.

engahmeds3ed commented 3 years ago

After searching I got that there is an issue on IOS with Safari related to using touchstart or touchmove without using touchend which make the links double click issue ( you need to click on any point from the screen as first click then u can click on any link )

the following link tries to highlight the issue: https://stackoverflow.com/a/10340968

On customer site I made the following change and solved the issue: https://github.com/wp-media/wp-rocket/blob/7ec9d996f004707a57c4c8476090d5bd943051f0/assets/js/lazyload-scripts.js#L91-L99 replace touchmove with touchmove touchend replace touchstart with touchstart touchend

After checking with @hellofromtonya , she said:-

This particular issue will need more investigation to find out why. Rather than adding a patch to the code for this specific edge case, why do they have the issue and why does this specific combination fix it? I think the “why” here will be very valuable to ensure we don’t inject another problem for other customers.

so I will add needs: r&d label to be investigated.

webtrainingwheels commented 3 years ago

Another case: https://secure.helpscout.net/conversation/1296370691/198532?folderId=377611 Customer reports it happening on chome and safari on mobile

vmanthos commented 3 years ago

Related ticket: https://secure.helpscout.net/conversation/1296809928/198688/

vmanthos commented 3 years ago

Related ticket: https://secure.helpscout.net/conversation/1322836283/206594?folderId=2135277

girlie commented 3 years ago

Related ticket: https://secure.helpscout.net/conversation/1334490646/210819/

NataliaDrause commented 3 years ago

Related ticket: https://secure.helpscout.net/conversation/1336672544/211528/ Notes and screen recording in the notes. The issue happens in both Safari and Chrome browsers.

Roboonl commented 3 years ago

After searching I got that there is an issue on IOS with Safari related to using touchstart or touchmove without using touchend which make the links double click issue ( you need to click on any point from the screen as first click then u can click on any link )

the following link tries to highlight the issue: https://stackoverflow.com/a/10340968

On customer site I made the following change and solved the issue: https://github.com/wp-media/wp-rocket/blob/7ec9d996f004707a57c4c8476090d5bd943051f0/assets/js/lazyload-scripts.js#L91-L99

replace touchmove with touchmove touchend replace touchstart with touchstart touchend After checking with @hellofromtonya , she said:-

This particular issue will need more investigation to find out why. Rather than adding a patch to the code for this specific edge case, why do they have the issue and why does this specific combination fix it? I think the “why” here will be very valuable to ensure we don’t inject another problem for other customers.

so I will add needs: r&d label to be investigated.

I tried this suggestion but it unfortunately didn't resolve the issue on iOS (for me at least).

BradFD commented 3 years ago

Same problem. Tried this feature for the first time a few days ago and got immediate reports that links on iOS weren't working and they needed double tap to work. Tried on 2 x iOS 14 devices and confirmed this problem so disabled for now.

Roboonl commented 3 years ago

Any updates on this?

NataliaDrause commented 3 years ago

Related: https://secure.helpscout.net/conversation/1367506767/223234

veganwebagency commented 3 years ago

Same problem. Any updates?

kashishkumawat commented 3 years ago

Experiencing the same bug with both WP-Rocket and Flying Scripts. Replacing touchstart and touchmove didn't help.

jorgemartine00 commented 3 years ago

Related: https://secure.helpscout.net/conversation/1376564315/226026/

socialpreneur commented 3 years ago

I have this happening on iOS Chrome browser rather than Safari. Anyone else can confirm?

Roboonl commented 3 years ago

We've dropped the WP Rocket plugin for now, this issue is already open for 3 months and I don't feel like it's going to be fixed any time soon with the current open issue count (+400).

NataliaDrause commented 3 years ago

Related: https://secure.helpscout.net/conversation/1379195337/226677

roberthedlund commented 3 years ago

Hope WP-rocket add the "fix" soon :)

veganwebagency commented 3 years ago

Any update from the developers? Are you working on it?

arunbasillal commented 3 years ago

@wp-media/dev Since we are seeing more and more of this and since the bug makes the feature quite unusable, I think we need to prioritise it. I have added it to the next sprint to see if we can figure out a solution. 🙏

roberthedlund commented 3 years ago

@wp-media/dev Since we are seeing more and more of this and since the bug makes the feature quite unusable, I think we need to prioritise it. I have added it to the next sprint to see if we can figure out a solution. 🙏

Sounds good. This feature make a big difference for my website when it comes to speed, would love to have it activated again =)

Tabrisrp commented 3 years ago

@arunbasillal no, the issue is marked as need R&D, it can't be part of a sprint. For an issue to be part of a sprint, it needs to be fully groomed first.

veganwebagency commented 3 years ago

@wp-media/dev Since we are seeing more and more of this and since the bug makes the feature quite unusable, I think we need to prioritise it. I have added it to the next sprint to see if we can figure out a solution. 🙏

Thanks! I really love this feature - would be awesome if it can be used on all my websites!

Tabrisrp commented 3 years ago

@wp-media/productrocket Some feedbacks from the team about this:

As there is no fix for that without significant changes to the current way this feature works, it needs further thinking before going further.

Garnet-Fox commented 3 years ago

Need and mouseWheel event and setTimeOut. On mouseweel scripts like Infinite Scroll and others scripts, works not correct, not loading next page if only mousewhell. In general, this applies to all scripts, and it critical. Delay JS without setTimeOut also cause problems, in addition to links needs to be clicked twice. Exemple: In the Russian segment, Yandex search engine. The counter can use a webviewer, this is a record of user actions. Accordingly, the script should not be delayed: since when viewing user actions, the counter script on the site should be loaded. And it naturally does not load because no actions, no 'keydown', 'mouseover', 'touchmove', 'touchstart' occur in the viewer.

socialpreneur commented 3 years ago

I'd really like to know how WP Rocket team is considering this, but at least auto load scripts after x seconds option is a must until iOS fixes the issue.

veganwebagency commented 3 years ago

@wp-media/productrocket Some feedbacks from the team about this:

  • This is tricky because of the way iOS works.
  • The thing is touchscreens don’t really have a good hover alternative, other than maybe touch-and-hold — but at least on iOS devices, that has its own significance (it loads a preview frame of the link destination).
  • Ahmed deeply dived into this also before, the only solution is to use timeout like flying scripts

As there is no fix for that without significant changes to the current way this feature works, it needs further thinking before going further.

Any timeline for the implementation for the timeout solution?

GeekPress commented 3 years ago

Loading scripts after X seconds isn't the solution. Doing that is just in the opposite of the interest of this feature. If we can find a solution without doing that, we will try to go on this way.

@arunbasillal @engahmeds3ed Could you redo some tests?

I'm asking as we had the same problem with our website too. But since last week, we don't have the problem anymore. Is it the coincidence of releasing our new website, or an iOS update?

Any timeline for the implementation for the timeout solution?

We can't provide any ETA.

socialpreneur commented 3 years ago

@GeekPress When I view your site with Chrome using mobile window size provided by Dev tool (Is there any official name for this feature? Perhaps mobile preview?), two finger scrolling never triggers delayed scripts including analytics. And for the double click problems on this thread, which page and link did you test? The top page?

Roboonl commented 3 years ago

@GeekPress When I view your site with Chrome using mobile window size provided by Dev tool (Is there any official name for this feature? Perhaps mobile preview?), two finger scrolling never triggers delayed scripts including analytics. And for the double click problems on this thread, which page and link did you test? The top page?

I don't think testing with the Mobile Preview in the Chrome Developer Tool gives the same result as actually testing on an iOS device due to the difference in touch gestures.

socialpreneur commented 3 years ago

@Roboonl You are right, they should be different. And I'm not doing so. I simply wonder if it is OK for WP Rocket that scripts are never loaded under certain condition(s).

Garnet-Fox commented 3 years ago

The setTimeOut is needed, or rather necessary.

NataliaDrause commented 3 years ago

related: https://secure.helpscout.net/conversation/1406743456/234374/

sandyfigueroa commented 3 years ago

Related case: https://secure.helpscout.net/conversation/1405169718/233859/

Browser: Safari and chrome Device: iPhone 11 Pro

piotrbak commented 3 years ago

Adding timeout trigger would also resolve https://github.com/wp-media/wp-rocket/issues/3420

GeekPress commented 3 years ago

Adding timeout trigger would also resolve #3420

@piotrbak Not sure #3420 is related to this problem. Fixing #3454 should I think fix #3420

piotrbak commented 3 years ago

@GeekPress From my understanding, the users were scrolling around on the webpage and the Delay JS wasn't triggered. The Google Analytics script was not loaded then.

Seems that you're right about the https://github.com/wp-media/wp-rocket/issues/3454 it's also related to mobiles. Probably any of those issues would resolve the https://github.com/wp-media/wp-rocket/issues/3420 Thanks

roberthedlund commented 3 years ago

Any update?

roberthedlund commented 3 years ago

@arunbasillal Cant you add an option in WP-rocket for this to exclude on certain pages instead in the meanwhile? Like Flying Scripts has, they even have option to exclude the Delay JS for certain pages with keywords.

alfonso100 commented 3 years ago

another case: https://secure.helpscout.net/conversation/1417928403/237257?folderId=2683093

GeekPress commented 3 years ago

I don't think testing with the Mobile Preview in the Chrome Developer Tool gives the same result as actually testing on an iOS device due to the difference in touch gestures.

@Roboonl It has been tested on a real mobile device. Please try our website on mobile, you don't need to click two times.

@roberthedlund I should have missed something, I don't find any way with Flying Script to have keywords based on pages.

roberthedlund commented 3 years ago

I don't think testing with the Mobile Preview in the Chrome Developer Tool gives the same result as actually testing on an iOS device due to the difference in touch gestures.

@Roboonl It has been tested on a real mobile device. Please try our website on mobile, you don't need to click two times.

@roberthedlund I should have missed something, I don't find any way with Flying Script to have keywords based on pages.

https://ibb.co/qYVyhh6

GeekPress commented 3 years ago

@roberthedlund Well, you can already do that with WP Rocket too. You have just to go on your page edit screen, open the « WP Rocket Options » metabox, and turn off the Delay JS Execution option.

roberthedlund commented 3 years ago

@roberthedlund Well, you can already do that with WP Rocket too. You have just to go on your page edit screen, open the « WP Rocket Options » metabox, and turn off the Delay JS Execution option.

Yeah but not ideal to do that manually on 100+ pages for an example.

veganwebagency commented 3 years ago

Any update?

socialpreneur commented 3 years ago

This issue is still available with version 3.8.6 with Chrome on Android. I just tried Fying Scripts, then this double click bug didn't happen. For iOS, even with Flying Scripts, viewers are required to double click.

roberthedlund commented 3 years ago

Any update? I find it strange that you have still not informed the users of WP-rocket in any way about the problem. How difficult is it to enter a warning notice before people activate the Delay JS feature? Have seen so many users in various forums who do not yet know that they are suffering from the problem before I have contacted them about it. It's a pretty big problem and I think you should warn about it.

GeekPress commented 3 years ago

@socialpreneur For iOS, are you trying on a real mobile, and not with a simulation?

Why I'm asking the question?

I retry again myself on my personal iPhone 8 and I don't have to click twice on all websites I tried which had Delay JS including our own website.

socialpreneur commented 3 years ago

@GeekPress On real mobile, including me and my client's phone, so I had to disable Delay JS completely. Flying Scripts didn't also work, so I'm guessing this is iOS limitation right now.

Added: It looks like there's some external or internal script which is causing this problem. Not every site which uses Delay JS of WP Rocket nor Flying Scripts have this double click symptoms. So far two sites that do not have this problem do not use adsense. I think we'll need to gather more cases to figure out which is the cause.

roberthedlund commented 3 years ago

@GeekPress

@socialpreneur For iOS, are you trying on a real mobile, and not with a simulation?

Why I'm asking the question?

I retry again myself on my personal iPhone 8 and I don't have to click twice on all websites I tried which had Delay JS including our own website.

I had tried on plenty of websites that using Delay JS, and have tried on real phones. iPhone 8, iPhone SE, iPhone 11 and all of them had this problem. Sometimes it workes on first "click" then the others totally stop working. So the issue is real.