One minor possible thing to fix here: currently (= before the patch too) links that do not get page previews still get their title attribute removed. For example, it can be pages in other namespaces linked from body text, like in articles about Wikipedia itself. Can there maybe be some code to restore those on :hover? (That would require storing title somewhere though, so I’d understand if it’s not worth the effort.)

This appears to have caused a regression: T269873

Wecan revert this change if necessary given we are coming up to the holiday period that might be the best thing to do here.

Change 648270 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/Popups@master] Revert "Remove title attributes at init"

https://gerrit.wikimedia.org/r/648270

Change 648270 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Revert "Remove title attributes at init"

https://gerrit.wikimedia.org/r/648270

The patch has been reverted for now. Feel free to restore it and we'll have another go at review.
@Nomsterio @Gilles per input on T269873 I think at minimum we need to not remove title attributes on links without a page preview. A possible trade off might be to remove them on hover but not restore them.

I'm not sure if the performance optimization actually improves performance. It probably doesn't have a noticeable effect either way. But I would expect removing a single attribute on hover to be basically instant, while removing potentially thousands of attributes on page load to be maybe noticeable.

Actually… Popups has a noticeable impact on the page load of large pages. For example, here it is in Chrome's performance inspector recording the page load of https://en.wikipedia.org/wiki/Donald_Trump, where it takes 600ms out of ~10s taken to load the page:

I haven't compared the before-and-after, but my instinct is to blame this on the reverted code.

Yes, it's possible.

In T269297#6687776, @matmarex wrote:

I'm not sure if the performance optimization actually improves performance. It probably doesn't have a noticeable effect either way. But I would expect removing a single attribute on hover to be basically instant, while removing potentially thousands of attributes on page load to be maybe noticeable.

In T269297#6687777, @matmarex wrote:

Actually… Popups has a noticeable impact on the page load of large pages. For example, here it is in Chrome's performance inspector recording the page load of https://en.wikipedia.org/wiki/Donald_Trump, where it takes 600ms out of ~10s taken to load the page:

I haven't compared the before-and-after, but my instinct is to blame this on the reverted code.

This was part of a general effort to reduce style thrashing on hover, that comes from a lot of little avoidable DOM changes like this one.
It's possible that on certain pages doing a lot of this at the start creates a different problem, which would make this solution counterproductive.
I think the optimal solution for this would come from the web platform, if calling preventDefault() on mouseover would turn off the default tooltip, a conversation that's currently happening.

Change 649408 had a related patch set uploaded (by Jdlrobson; owner: Bartosz Dziewoński):
[mediawiki/extensions/Popups@wmf/1.36.0-wmf.21] Revert "Remove title attributes at init"

https://gerrit.wikimedia.org/r/649408

Change 649408 merged by jenkins-bot:
[mediawiki/extensions/Popups@wmf/1.36.0-wmf.21] Revert "Remove title attributes at init"

https://gerrit.wikimedia.org/r/649408

I looked again now after the revert has been deployed, there's an obvious difference:

In T269297#6687847, @Nomsterio wrote:

This was part of a general effort to reduce style thrashing on hover, that comes from a lot of little avoidable DOM changes like this one.
It's possible that on certain pages doing a lot of this at the start creates a different problem, which would make this solution counterproductive.
I think the optimal solution for this would come from the web platform, if calling preventDefault() on mouseover would turn off the default tooltip, a conversation that's currently happening.

Yeah, it looks like doing this on load turns out to be a problem. Changing the behavior of preventDefault() sounds interesting, I think I like that.

@Gilles @Krinkle what is the performance recommendation here, given @matmarex and the revert that happened here?
Am moving this off our sprint board (and thus radar) for now until we can get clearer guidance on whether we still want to do this, and what we want to do differently.

In T269297#6745086, @Jdlrobson wrote:

@Gilles @Krinkle what is the performance recommendation here, given @matmarex and the revert that happened here?
Am moving this off our sprint board (and thus radar) for now until we can get clearer guidance on whether we still want to do this, and what we want to do differently.

From my point of view, I'm trying to follow up the spec route and see if something can be done with preventDefault().
For wikimedia at this stage, I suggest avoiding the title attribute if possible, but I don't know the repercussions and will leave that to the CC'ed :)

I think that a potential workaround to this gadget situation would be to expose the current contents of the title attribute via a new custom attribute, and inform gadget maintainers that the title attribute is going to be deprecated at point X in the future, in favor of this new attribute that will contain the same data.

Then we can consider either removing the title attribute after init as originally planned here or removing the title attribute for those links altogether.

The title attribute is only tangentially used by gadgets. This is neither documented, nor encouraged or supported as far as I'm concerned.

First and foremost it is for humans interacting with the link. I think it would be a mistake to unconditionally remove upfront when Popups is active. It would allow for no fallback if some of its code fails or becomes inactive in some cases. I think there are also other cases besides mouse hover where this attribute is used, such as accessibility tools going through the page, long press on mobile, and various other mechanisms.

Also, I would expect that modifying thousands of anchor elements in the DOM during the critical path up front would be rather expensive, I don't understand how that can be efficient. Is it?

Removing and restoring one attribute around a manual hover interaction seems super cheap and should not be causing performance issues. Even if removing the attribute took 10 milliseconds (which it doesn't), would that even be an issue? Both the browser tooltip and our own hover card are intentionally delayed by several hundred milliseconds. How does the user perceive this delay? Note that we are (afaik) making a write-only modification that does not force a repaint of any kind so it wouldn't be lowering the framerate either, but just take up some time on the JS thread when we're not really doing anything else there.

In T269297#6756320, @Krinkle wrote:

Also, I would expect that modifying thousands of anchor elements in the DOM during the critical path up front would be rather expensive, I don't understand how that can be efficient. Is it?

It isn't, see my earlier comments T269297#6687777 and T269297#6690105.

Thanks <3, I totally missed that.

Yes, the solution of modifying on init was obviously not the right solution :)

I do recommend, however, to discourage use of the title attribute in the future. It has been considered an anti-pattern in terms of accessibility for several years.

It's true that looking at the on-hover removal of the title attribute in isolation it would seem innocent, but when I work on optimizing interaction-driven paths, I look for creeping normalities - a bunch of those innocent avoidable DOM changes, which end up adding up, causing unexpected layout/style thrashing and it's difficult to name a particular one of them as a performance culprit. So I encourage reducing JS-driven DOM changes on hover & other interactions to a minimum (none if possible). Of course, if a particular JS DOM change on hover is the best way to achieve something and the alternatives are too expensive, it would make sense to keep it, and I hope in the future my preventDefault proposal or some other browser improvement render it unnecessary.

Performance team: it's unclear what the ask is from our team right now. Please feel free to tag readers web backlog, if there is a clearer performance team recommendation here.