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
#4
@
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
@
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
@
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
#9
@
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
@
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
#16
in reply to: ↑ 15
;
follow-up:
↓ 18
@
2 years ago
- Keywords close added
This ticket was mentioned in Slack in #accessibility by dennysdionigi. View the logs.
2 years ago
#19
@
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
@
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
@
4 months ago
- Milestone Awaiting Review → 7.1
- Owner set to
- 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
- 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). Soclicknow fires promptly;touchstartbuys nothing. <a>taps already fireclick. The reply links are anchors, so a touchscreen tap generates aclickevent on its own (a commenter on the ticket makes exactly this point). Thetouchstartbinding is pure redundancy.touchstartis 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
@
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:
↓ 25
@
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
@
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.
@westonruter commented on PR #12168:
4 months ago
#26
Before 👎🏻 | After 👍🏻
<video src="https://github.com/user-attachments/assets/ef7e1905-a80d-4dfe-aa22-7d9537966daa"> | <video src="https://github.com/user-attachments/assets/e28c09bf-37b9-4d78-942e-da5143b4d9f8">
#28
@
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”.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
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