Make WordPress Core

Opened 8 years ago

Closed 4 months ago

#46713 closed defect (bug) (fixed)

Comment reply link uses inconvenient eventlistener

Reported by: szandman Owned by: westonruter
Priority: normal Milestone: 7.1
Component: Comments Version: 5.1
Severity: normal Keywords: has-patch has-screenshots commit
Cc: Focuses: ui, accessibility, javascript, performance

Description

When comment-reply.js was rewritten recently the behavior of the reply-button changed. It now seems to use 'touchstart' as eventlistener which makes it really "touchy" to use on mobile. It's almost impossible to use scroll now since you always trigger the touchstart event which opens the reply-box.

Change History (29)

This ticket was mentioned in ​Slack in #core-editor by peterwilsoncc. ​View the logs.


8 years ago

This ticket was mentioned in ​Slack in #core-js by peterwilsoncc. ​View the logs.


8 years ago

#3 @janpaulkleijn
7 years ago

Hi there,
During my efforts to make my website more accessible I was also pointed out by Google LightHouse that the adding of the 'touchstart' event listener in this file lacked the option passive.

This can be corrected by altering the file: wp-includes/js/comment-reply.min.js

First the availability of support for the option parameter in the User Agent must be checked.
​https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Safely_detecting_option_support

Check option support (minified)

var supportsPassive=false;try{var opts=Object.defineProperty({},'passive',{get:function(){supportsPassive=true;}});window.addEventListener("testPassive", null, opts);window.removeEventListener("testPassive", null, opts);} catch (e) {}

So you insert this code in the comment-reply.min.js file, right above the current code.

Then the option must be added to the call for the 'touchstart' event listener in the code itself. Where you see:

addEventListener("touchstart",f)

you should replace it with:

addEventListener("touchstart",f,supportsPassive?{passive:true}:false)

At the moment this event listener is found only twice in the beginning part of the code.

This will solve your issue.

A good and thorough explanation of passive event listeners:
​https://github.com/WICG/EventListenerOptions/blob/gh-pages/explainer.md

#4 @Znuff
7 years ago

@janpaulkleijn thanks for the suggested fix, our users were complaining about hitting the reply button while scrolling continuously, we couldn't figure out what it was initially, as it's not exactly obvious at first.

#5 @janpaulkleijn
7 years ago

No problem. It's true that these days an incredible lot happens on a page, which makes looking for the culprit not always that easy. It's kind of a detective thing...

#7 @afercia
7 years ago

Introduced in [42360] / #31590. Details on the Chrome / Lighthouse warning mentioned in #47510: ​https://developers.google.com/web/tools/lighthouse/audits/passive-event-listeners

#8 @eherman24
7 years ago

Patch attached to #47510

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

#9 @madhazelnut
6 years ago

Do the passive listeners "fixes" actually address the core of this ticket? WordPress is still (5.4.2) displaying the same issue on mobile.

This ticket was mentioned in ​Slack in #core by madhazelnut. ​View the logs.


6 years ago

#11 @peterwilsoncc
6 years ago

As I've mentioned in #47510, passive listeners are incompatible with the event handler as it calls Event.preventDefault() -- please continue any discussion on moving to a passive listener on the existing ticket.

This ticket should focus on the reported issue of whether thouchstart is the appropriate place to add the event listener.

This ticket was mentioned in ​Slack in #core by peterwilsoncc. ​View the logs.


6 years ago

This ticket was mentioned in ​PR #2818 on ​WordPress/wordpress-develop by ​jdevalk.


4 years ago
#13

  • Keywords has-patch added

Brought to light by recent Lighthouse updates, we should make the touchStart events in our comment reply script passive.

See ​https://web.dev/uses-passive-event-listeners/ for background and explanation.

This might affect (and partially fix) https://core.trac.wordpress.org/ticket/46713

Trac ticket: https://core.trac.wordpress.org/ticket/55988

#14 @westonruter
3 years ago

  • Focuses performance added

#15 follow-up: @Znuff
3 years ago

I love how this is 4 years old and... nothing happened yet.

#16 in reply to: ↑ 15 ; follow-up: @pbearne
2 years ago

  • Keywords close added

Replying to Znuff:

I love how this is 4 years old and... nothing happened yet.

It looks like this is mostly fixed in #55988

I am close this I don't think any more work will done on this ticket

Last edited 15 months ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in ​Slack in #accessibility by dennysdionigi. ​View the logs.


2 years ago

#18 in reply to: ↑ 16 @SergeyBiryukov
15 months ago

  • Keywords close removed

Replying to pbearne:

It looks like this is mostly fixed in #55988

#55988 didn't have any changes committed to core, so that does not appear to be the case.

#19 @edent
4 months ago

  • Focuses ui accessibility javascript added

Another report from a real user affected by this bug - ​report

It is primarily a problem for left-handed people. When they scroll using their left thumb, it is more likely to trigger the touch behaviour than people scrolling with their right thumb. The "reply" link is usually left-aligned.

This bug impacts about 10% of the population - ​https://www.bbc.com/future/article/20160930-the-mystery-of-why-left-handers-are-so-much-rarer

I'm not sure why a "touchstart" event is necessary here. A touchscreen user tapping on the link will generate a "click" event.

I propose removing the "touchstart" from the code.

It was introduced in 2017 as a way to modernise the detection of events.

​https://github.com/WordPress/wordpress-develop/commit/14dd35e62e1325ca1c6caef0a8623ce3f02d7da9

It was probably useful then, but now causes problems.

This ticket was mentioned in ​PR #12168 on ​WordPress/wordpress-develop by ​@edent.


4 months ago
#20

This touch event frequently triggers erroneously for left-handed users when scrolling. In serves no purpose in the 2020s; touchscreen users tapping on a link generate a click event.

Trac ticket: https://core.trac.wordpress.org/ticket/46713

## Use of AI Tools

AI assistance: No
Tool(s): My brain
Model(s): Late 70s model, well maintained, some cosmetic wear.
Used for: Writing down the problem. Thinking about the problem. Writing the solution.

#21 @afercia
4 months ago

Rather than performance (likely false) positives warnings, I'd be more concerned about the user experience. Triggering an unwanted UI change when users only want to scroll the page on mobile doesn't seem to be the best user experience.

I read some of the relevant discussions and references:
https://core.trac.wordpress.org/changeset/42360
https://core.trac.wordpress.org/ticket/31590
​https://github.com/WordPress/wordpress-develop/pull/2818
https://core.trac.wordpress.org/ticket/47510

I'm not sure I understand why there's the need for a touchstart event in the first place. What's wrong with using only a click event?

The problem with touchstart is that it fires while touching. As such, when scrolling the page on mobile if users happen to try to scroll and touch the Reply or Cancel links, the comment UI will move and show the comment form. That's not what users would expect.
Instead, the click event is not fired when scrolling.

Would just removing the touchstart events be an option? Cc @peterwilsoncc

#22 @westonruter
4 months ago

  • Milestone Awaiting Review → 7.1
  • Owner set to westonruter
  • Status new → reviewing

I agree that the touchstart event handler should be removed in favor of just leaving click event handler instead. I remember long ago that using touchstart was a way to eliminate an added 300ms delay to running the event handler, because the browser had to wait to figure out if it is a double-tap. But as of 2013, this is no longer an issue: ​300ms tap delay, gone away.

This will also fix the issue with Lighthouse complaining about the touchstart event not being passive, since the event handler is removed entirely.

The correct solution appears to be ​https://github.com/WordPress/wordpress-develop/pull/12168


Here's additional Claude's analysis of the history of this ticket and the origin of the touchstart event, and why it's no longer needed.

Why touchstart was used in the first place

It was added in [42360] / #31590 ("Comments: Modernise JavaScript for comment reply links," Peter Wilson, Dec 2017 — shipped in 5.1.0), the rewrite that converted the old inline onclick="return addComment.moveForm(...)" attributes into unobtrusive addEventListener handlers. Both the cancel link and each reply link got bound to both events:

element.addEventListener( 'touchstart', clickEvent );
element.addEventListener( 'click',      clickEvent );

Notably, there's no documented rationale for touchstart in #31590 — it was just included as part of the rewrite. But it's a well-understood idiom of that era: the ~300ms "click delay" on mobile browsers. Historically, touch browsers waited ~300ms after a tap before firing the synthetic click, so they could detect a double-tap-to-zoom. Binding touchstart was the common "fast-click" trick (the FastClick library era) to make taps feel instant by reacting at the start of the touch instead of waiting for the delayed click.

Why it's safe — and correct — to remove it now

  1. The 300ms delay is gone. Modern mobile browsers eliminated it for responsive pages (those with <meta name="viewport" content="width=device-width">) — Chrome/Android since ~2014, iOS Safari since 9.3 (2016). So click now fires promptly; touchstart buys nothing.
  2. <a> taps already fire click. The reply links are anchors, so a touchscreen tap generates a click event on its own (a commenter on the ticket makes exactly this point). The touchstart binding is pure redundancy.
  3. touchstart is actively harmful here. It fires at the beginning of a touch — before the browser can tell a tap from a scroll. So when a user's thumb lands on the left-aligned "Reply" link while scrolling, the reply box snaps open. That's the "touchy" bug, and it disproportionately hits left-handed users.

So removing the two touchstart lines (keeping click) is the right fix.

One thing to keep separate

Don't conflate this with the passive listener angle from the related tickets (#47510, #55988, and a Lighthouse warning). Those want to mark the touchstart listener { passive: true } — but as Peter Wilson noted on the ticket, that's incompatible here because clickEvent/cancelEvent call Event.preventDefault(). Removing touchstart entirely sidesteps that whole problem too, since the only remaining listener is click.

#23 @afercia
4 months ago

Oh, interesting. Thanks for clarifying the original reason why the touchstart events were originally introduced @westonruter. I did search for it but couldn't find it. +1 from my side to remove these events, glad to see this ticket unblocked.

#24 follow-up: @edent
4 months ago

Your LLM has gotten this wrong.

The blog post you mention was published in 2013. The original change was added in 2017. I'm not a super advanced AI, but I'm pretty sure my mental arithmetic shows that the change was added some 4 years *after* that post.

Even back in 2015, it had become obvious that the 300ms delay ​was no longer a live concern on most browsers.

Once again, Claude has added no value and has only served to confuse. Remember, AI is no substitute for actually thinking about the problem.

I'm grateful that you have accepted my PR. I'm somewhat bemused that it has taken 7 years to fix from the initial report.

#25 in reply to: ↑ 24 @westonruter
4 months ago

Replying to edent:

The blog post you mention was published in 2013. The original change was added in 2017. I'm not a super advanced AI, but I'm pretty sure my mental arithmetic shows that the change was added some 4 years *after* that post.

Even back in 2015, it had become obvious that the 300ms delay ​was no longer a live concern on most browsers.

Well, browsers don't update immediately, and best practices tend to lag as developers adopt new practices. WordPress moves even slower. I don't think it's unreasonable to conclude that touchstart was added as a carryover from that time.

Once again, Claude has added no value and has only served to confuse. Remember, AI is no substitute for actually thinking about the problem.

I did think about it. I shared as a cross-reference. It mostly aligns with my history of events.

Last edited 4 months ago by westonruter (previous) (diff)

#27 @westonruter
4 months ago

  • Keywords has-screenshots commit added

#28 @westonruter
4 months ago

As I just commented on #47510, note that ​Lighthouse 13 (Oct 10, 2025) removed the ​uses-passive-event-listeners audit, reasoning:

Rarely an issue in first-party scripts these days, and it is inactionable for third-party scripts that use this.

So removing the touchstart event isn't going to “feed two birds with one scone”.

#29 @westonruter
4 months ago

  • Resolution → fixed
  • Status reviewing → closed

In 62511:

Comments: Remove touchstart event listeners from comment reply/cancel links.

The touchstart event on comment reply and cancel links fired before the browser could distinguish a tap from a scroll gesture, causing the reply form to open unexpectedly during scrolling. Since touchscreen users tapping a link already generate a click event, the touchstart listeners are unnecessary. Prior to ~2015 a touchstart event may have been useful to eliminate a 300ms delay for event handlers on tap, but this is long obsolete.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/12168.
Follow-up to r42360.

Props edent, szandman, SergeyBiryukov, westonruter, afercia, peterwilsoncc, janpaulkleijn, madhazelnut, joostdevalk, pbearne, eherman24, Znuff.
See #47510, #31590.
Fixes #46713.

Note: See TracTickets for help on using tickets.