FIX: Make sure user preference to open external links in new tab works for bookmark list excerpts (PR #10409)

Meta post: https://meta.discourse.org/t/bookmark-page-does-not-respect-open-all-external-links-in-new-tab-user-preference/160118

GitHub

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/bookmark-page-does-not-respect-open-all-external-links-in-new-tab-user-preference/160118/4

could use addEventListener

scheduleOnce is useless here, because you pass it an anonymous function, so even if it was called multiple times (which is probably not) it’s still considered a new function each time and running it again

can use native js

also it would be better in didInsertElement than init

@jjaffeux thanks for the review, you are right…I have used jQuery again where JS will do. Will change.

could probably be _boundClick don’t think this should be considered public

we don’t use setTimeout as much as possible, in your case schedule("afterRender", ... is probably the best choice

I think the best now is to use dataset https://developer.mozilla.org/fr/docs/Web/API/HTMLElement/dataset

maybe we could try to have a more specific selector than “tr” ?

you should probably check if link exist, given there’s a later, it’s possible that 50ms later link doesn’t exist anymore

why do we attempt to open something if href === “” ?

totally forgot to tell this, but we should remove this event listener

We should definitely do this TODO sooner rather than later. We should not be manipulating the dom in a controller.

Yes I think Martin is working on this.