Developer tool - Best Practice - input comment field Does not use passive listeners

[efilippucci]

Using chrome developer tool, I noticed that, when is displayed the input comment field in standard post, one best Practice is not satisfied:

Does not use passive listeners to improve scrolling performance.
Consider marking your touch and wheel event listeners as passive to improve your page’s scroll performance.
js/comment-reply.min.js?ver=5.2.1

One theme developer confirmed that this is a WordPress core problem: ​https://generatepress.com/forums/topic/gp-2-3-alpha-3-consider-using-passive-listeners/#post-923468

[SergeyBiryukov]

Related: #46713

[mwanmedia]

waiting this ticket

[anonymized_14934761]

This is hurting the wordpress performance. Please consider rising it up as a priority.

[eherman24]

Do we need a patch for this? I've also encountered this issue this morning and since it's outside of my control there is nothing I can do.

[eherman24]

Set event listeners to passive

[eherman24]

I've attached a patch which sets the event listeners to passive, and ultimately resolves the notice thrown in Lighthouse.

Before Patch:
​https://cldup.com/s1YO-ay-Gd.png

After Patch:
​https://cldup.com/wBq0Ha6LO4.png

Tests:
✅Lighthouse audits run using TwentyNineteen
✅Lighthouse audits run using TwentyTwenty

[anonymized_14934761]

Great, thank you!

[efilippucci]

Thanks a lot, any chance to put it in the next WordPress 5.3 release?

[eherman24]

@efilippucci I don't think that it will make the 5.3 release. That is slated for 11/12, which is only 3 days away. I'm guessing things are in a freeze for testing right now before that release, and (if the patch is accepted) it will make it into the next security release (5.3.1).

[soulkitchen]

Hello
your patch is for the normal js/comment-reply.js file. please consider that is this file: js/comment-reply.min.js (sorry if I say any wrong words or do not understand how things work. just a rookie I am. :) )
Thank you.
Best regards

Replying to eherman24:

@efilippucci I don't think that it will make the 5.3 release. That is slated for 11/12, which is only 3 days away. I'm guessing things are in a freeze for testing right now before that release, and (if the patch is accepted) it will make it into the next security release (5.3.1).

[andraganescu]

Hi @eherman24 the patch looks good but I checked and since we still support IE11 there might be a need to check if passive is supported.

​Can I use tells us IE11 doesn't support passive listeners and ​MDN advises we create some way to detect prior to setting the option.

Could you please add this to the patch?

[audrasjb]

Given this issue is not a regression caused by WordPress 5.3, let's handle this in the next major release, 5.4.

[anonymized_14934761]

In my opinion it is small enough and critical enough to improve the performance that I think it can be released in 5.3.1.

Is there a way I could apply the patch manually by downloading the files?

[audrasjb]

Hi @slkfsdf8y34ljhsfsdfkuhfkl84hj ,

Replying to slkfsdf8y34ljhsfsdfkuhfkl84hj:

In my opinion it is small enough and critical enough to improve the performance that I think it can be released in 5.3.1.

Unfortunately, 5.3.1 is scheduled for Thursday 12 with a Release Candidate at the beginning of the week, and this ticket still needs a patch that addresses the above issues (see previous comments).
To milestone it again to 5.3.1, we need a new patch by tomorrow and people available to test this patch. That could still happen but doesn't sounds very realistic to me.

Is there a way I could apply the patch manually by downloading the files?

Since it is a JS change, you'll need to run npm run build command on wordpress-develop after applying the patch on your development instance of WordPress. See related docs for reference: ​https://make.wordpress.org/core/handbook/tutorials/working-with-patches/

Cheers,
Jb

[chadrew]

After applying the patch, a new error appears in Chrome's developer tools console when clicking the reply link:

comment-reply.js:1 Unable to preventDefault inside passive event listener invocation.

Edit: there's also a change in behavior when you touch the reply link and immediately begin to scroll on mobile. Before the form wouldn't open, now it opens and you scroll away. Just an observation, no clue if this is undesirable.

[anonymized_14934761]

Since Windows 7 support will end on January 14, 2020 and after that Google will display full screen warnings and won't even ship security software to it, I think it is safe to say that WordPress should ditch IE11 support after Jan 2020.

There is no point to support a browser which won't be supported even by its creators.

[anonymized_14934761]

Replying to chadrew:

After applying the patch, a new error appears in Chrome's developer tools console when clicking the reply link:

comment-reply.js:1 Unable to preventDefault inside passive event listener invocation.

Any update on this?

[audrasjb]

Hi,

With 5.4 Beta 3 approaching and the Beta period reserved for bugs introduced during the cycle, this is being moved to Future Release. If any maintainer or committer feels this should be included or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[anonymized_14934761]

And this is how WordPress created a huge backlog that is growing bigger and bigger. It is preposterous that an issue of that scale is left to be postponed in that way in the future so many times.

[audrasjb]

Right, but to be fixed, the issue needs contributors to propose a patch.
The current patch needs to be refreshed first, and it's too late for 5.4, that's why this issue won't be fixed in the incoming release, unfortunately.

[anonymized_14934761]

I understand, but that was said the previous time as well. I am sure something like that would happen in a next version as well and so on. The old tickets are growing.

To be honest as a side viewer of all this, I think everything is not streamlined. If I was someone responsible for WordPress.org I would significantly streamlined and simplify everything, including the structure of the site, so it gets as minimal as possible and every focus is on developing.

I think the whole website needs to become significantly simpler, streamlined, modernized and these tickets should be put in front of everything.

Currently what's happening is bureaucracy building more bureaucracy leading to more and more backlog.

If the environment is good, then the participants would be more active.

[peterwilsoncc]

Both the reply and cancel button event handlers use event.preventDefault(); so switching to passive handlers can't be made without other refactoring.

I can't see how the event handlers can be rewritten to avoid event.preventDefault(); for a couple of reasons:

• without preventing the default action, clicking the links will cause them to be followed by the browser
• backward compatibility with custom comment engine plugins overwriting the moveForm() function

While a high lighthouse score feels nice, it's an automated score-card. Changes such as this would need to come with substantiated performance increases.

[anonymized_14934761]

The current state of the WordPress development:

• WordPress.org - outdated site with cluttered sections;

• Tickets - pilling up more and more while less and less are resolved;

• WYSIWYG editing that Web.com, Wix, Squarespace, Shopify did comes to Gutenberg which is 1/10th of the way there far too little, far too late.

• Static mindset - "things are good the way they are and should stay like that for compatibility reasons, aka Windows-y mindset.

I am sorry I don't see a future in this CMS in the longrun, Squarespace, Wix and Shopify would take even more market share because of this WRONG anti-consumer mindset.

P.S. IE11 was released 7 years ago, it's primarily used for oldschool banking and government entities since Microsoft moved to Edge, now Edge is even re-released with the same name as a brand and with Chromium as an engine. IE11 is just a mode inside Edge now on Windows platforms. Nobody would use that mode apart from some old government site that was still not updated. It doesn't make any sense for us to seek for such compatibility. Nothing works properly on the IE11 anymore, it's just a legacy mode for just few sites out there that large institutions cannot change just now.

My whole point is - let's move forward!

[anonymized_14934761]

This ticket was opened 9 months ago. The pile of tickets is growing more and more and less is resolved. What's needed and what are we waiting for? AI to solve all of them? It's disastrous, while in the meantime Squarespace, Wix, Shopify and others are having clean, simple, streamlined WYSIWYG editing that makes a website popups up in seconds.

I am starting to think WordPress is not competitive in the long run anymore!

[peterwilsoncc]

@slkfsdf8y34ljhsfsdfkuhfkl84hj please refrain from snarky comments on trac and WordPress.org generally.

I'm happy for this to move this ticket forward if I am missing how these functions can be rewritten without relying on event.preventDefault() and a performance gain can be shown.

[anonymized_14934761]

We have internal split tests of ranking advantages in the search engines before/after optimization of the Lighthouse scores are made.

[anonymized_14934761]

What kind of action is needed for this to move forward and not become one of the thousands opened 10+ year old tickets that are just piling up?

[peterwilsoncc]

Replying to slkfsdf8y34ljhsfsdfkuhfkl84hj:

What kind of action is needed for this to move forward....?

In order to use passive listeners the handlers need to be written in a way that does not rely on event.preventDefault(). I don't think this is possible as the handlers prevent the browser from following the link (a non-JavaScript fallback) but I'm happy to hear from other contributors.

Lighthouse is an automated tool that warns based on rules of thumb. By avoiding the need to visit a new page to reply to a comment, the commenter is able to reply immediately rather than waiting for a new page to load and render.

I'd caution against blindly following the recommendations of any automated tool, as it can ​lead to problems in itself.

As I said, if the listeners can be rewritten so they don't need event.preventDefault() without removing the fallback then I am happy for this to move forward.

[anonymized_14934761]

"In order to use passive listeners the handlers need to be written in a way that does not rely on event.preventDefault(). "

Otherwise it won't be compatible with the minority of IE11 users, otherwise it would be working as expected?

I did some tests with the lighthouse scores in terms of ranking and I found that on many occasions it helped rankings overall.

The current situation is decreasing the scores a lot.

[peterwilsoncc]

Replying to slkfsdf8y34ljhsfsdfkuhfkl84hj:

"In order to use passive listeners the handlers need to be written in a way that does not rely on event.preventDefault(). "

Otherwise it won't be compatible with the minority of IE11 users, otherwise it would be working as expected?

I did some tests with the lighthouse scores in terms of ranking and I found that on many occasions it helped rankings overall.

The current situation is decreasing the scores a lot.

This is unrelated to IE11 support, per the ​MDN addEventListener documentation using a passive listener will cause the event to fail and throw a console error if it contains event.preventDefault():

A Boolean which, if true, indicates that the function specified by listener will never call preventDefault(). If a passive listener does call preventDefault(), the user agent will do nothing other than generate a console warning.

[jr3074]

is there any patch?

[anonymized_14934761]

Is there a way to make the comments use passive listeners without creating issues or find another way to patch this performance issue?

[GDriver]

Any progress about those Passive Event Listeners?

[thegulshankumar]

Hi...

Consider fixing this issue.

[peterwilsoncc]

I've decided to close this off with the wontfix status as the code is incompatible with passive listeners.

As mentioned above, the event listener uses event.preventDefault() to stop browsers from following the link. Using a passive listener would result in browsers following the link.

Discussion can continue on closed tickets but unless browser behavior changes to allow both passive listeners and preventing the default action, the dev-tools report remains a false positive.

[butterflymedia]

This is more relevant now with Google's Page Experience and the Core Web Vitals. It really needs to be refactored.

If it's related to IE11, IE support is being removed, so it would be great if this feature would be added to 5.8.

If it's related to the actual JS code refactoring, could a <span> element be used instead of an <a> element and then it would solve @peterwilsoncc comment?

[peterwilsoncc]

@butterflymedia This is unrelated to the support of IE11 as noted above.

This is unrelated to IE11 support, per the ​MDN addEventListener documentation using a passive listener will cause the event to fail and throw a console error if it contains event.preventDefault():

A Boolean which, if true, indicates that the function specified by listener will never call preventDefault(). If a passive listener does call preventDefault(), the user agent will do nothing other than generate a console warning.

Unless a patch can be created that removes the need to prevent the default event, this remains unfixable in all browsers. If such a patch is provided, then it may be possible to reconsider this issue, otherwise the dev-tools report remains a false negative.

As this is unfixable I've reclosed the ticket but you are more than welcome to continue the discussion while the ticket is closed.

[peterwilsoncc]

#55988 was marked as a duplicate.

[alekv]

What about adding { passive: false } to suppress the warnings? event.preventDefault() will keep working.

I have an element that get's rerendered every second, which causes warnings to pile up. I'd like to remove those warnings.

On the other hand, if no warnings pop up anymore, this trac ticket will be forgotten.

[peterwilsoncc]

@alekv Yes, that could be done.