Modernizr.touchevents use can lead to scenarios that break contextual links

1: modernizrtouch-breaks-firefox-2168711-1.patch queued for re-testing.

I have a possible solution for this issue. It's not extremely pretty, but it's smart:

If we attach a touchstart event listener to the window object very early, this is the very first event handler that will be executed when (if) a user touches the screen. We initialize a variable that represents the presence of a touch capable screen(device) to false, and set it to true inside the event listener. Also, we can attach the event using one(), so that after it's triggered it unattaches itself. Code from the patch:

// Detect if screen is touch capable. This only happens at the first user
// interaction, so you can't depend on it when te page is renderd.
Modernizr.touchDisplay = false;
if (Modernizr.touch) {
  // touchstart is the first event triggered when a user touches the
  // screen, so this will be executed before any other listener that
  // needs to know if the device is touch-capable or not.
  jQuery(window).one('touchstart', function (){
    Modernizr.touchDisplay = true;
  });
}

The only problem with this solution is that at the time the page is rendering, the user most definitely did not interact yet with the page, so we can't know yet if the device has a touch capable screen or not. This limits our use of this detection mechanism to the event handlers, because they are invoked after the user started to interact with the page.

I have created a patch that implements this ideea, I just haven't been able to test this on a touch device, I will probably test this on simplytest.me sometimes later in the day.

P.S. I am not sure if adding a new attribute to the Modernizr object is the best approach, but it has the added benefit of being the first place where someone would look for this.

I managed to test this patch on my Nexus 10 in Chrome, and table drag is working. I don't know where the code from contextual.js is used, so that one is not tested on a mobile device. Also we need to test this in a desktop browser that reports it has support for touch events.

Then please also make sure to not create a UX regression in contextual links; we specifically excluded hover events there on touch-enabled devices because when you have both "hover" and "click" events on a certain DOM element, then "hover" equates to a single tap and "click" equates to a double-tap, which is an atrocious UX.

Needs work based on #8 I think.

Add a ref: http://blogs.msdn.com/b/ie/archive/2014/09/05/making-the-web-just-work-w...

Ok, I was taken here after a couple of duplicates :)

Thing is, contextual links don't even show up when I hover on my machine, on chrome. Firefox is fine.
Note that it's a touch enabled laptop.

Try it.

tableDrag, I think I've fixed it some times ago..

Let me explain it in a little further. Above patch works because `touchstart` event is fired before mouse event.

Ahh. I think I should add `mousemove` event to re-enable it again after any touch event

(this patch may cause little performance impacts)

Bump.

Issue still present on touch-enabled laptop. This problem won't go away.

@droplet

+++ b/core/modules/contextual/js/views/RegionView.js
@@ -16,16 +16,30 @@
+          istouchStart = true;

Is the expectation to completely disable the 'hover' behavior when interacting with the region?

@ChristianAdamski

Could you give the patch in #14 a shot?

The conditional if(!istouchStart = false;) followed by assignment: istouchStart = false; seems a bit overkill considering the variable is already checked to be false.

I really like that it's removing the Modernizr dependency!

Here's a patch for drupal 8.5

For this issue need to update core modernizr at first since it's outdated (v 3.3.1 currently).

Modernizr 3.7.1 got an update for touchevent tests. See here https://github.com/Modernizr/Modernizr/pull/2409 . This update was related to the latest versions of Chrome browser. So right now Modernizr.touchevents returns "true" always in chrome browser (???). These changes looks a bit weird for me, so maybe better to postpone an update for this issue, until we get how to detect "real" touch screens.

Hit the same issue as #26, using an updated version of modernizr, created from a build pipeline breaks contextual links in recent versions of chrome.

Quoted from the chrome platform status post:

Site should use navigator.maxTouchPoints for touchscreen detection, and Windows.TouchEvent for TouchEvent feature detection.

So in new versions of modernizr, the Modernizr.touch check is based on support for window.TouchEvent, which is supported in desktop versions of Chrome and not necessarily just touch devices. Forcing a downgrade to 3.3.1 should fix this, but a long term solution for core should be detecting touch devices not touch events.

Rerolled patch with reroll diff against 9.4.x branch.

If #34 is the solution then we can remove Modernizr from the dependencies of RegionView.js

Addressed #35 & added an interdiff.

I like the approach! The earlier patches remove some docs and formatting so I created a new patch that doesn't remove/restructure anything that can remain intact. The interdiff would be incoherent, so it's just a new patch. I also added some comments to explain what the logic is doing, and removed the Modernizr dependency entirely.

Tabledrag stopped using Modernizr ~6 years ago: #2177293: Drag and drop broken on touch enabled laptops. I updated the issue summary a bit.

I don't have a touch enabled laptop so I can't actually test this properly. But the code makes sense and is in line with #14 so that's a pretty good indication this should be the right solution :)

I tested the patch from #43 using a fresh install of the Standard profile on 10.0.x.

I am using a laptop with a touch pad and a touch screen. I did not see any difference between the contextual links with and without the patch. Either way, I could use the touch pad to hover and then click on a contextual link, or I could click on the Edit button and then click on the contextual links. Using the touch screen, I could use the Edit button and then the contextual links.

My laptop has a 13" screen, and the first time I tried to click on a contextual link using the touch screen (without the patch), I had trouble. I zoomed the screen a bit, and then it worked fine.

Removing needs manual testing tag based on #44.

Committed 10bdae2 and pushed to 10.0.x. Also committed to 9.5.x and cherry-picked to 9.4.x. Thanks!

Committed from 37000ft above Firebaugh, California, on the way home from DrupalCon Portland 2022.