[Bug] Closing a reddit comment thread redirects to homepage

[namazso]

Describe the bug
if you click the [-] button next to a reddit comment thread it will redirect you to homepage instead of closing.

Steps to Reproduce

1. click on [-] on any reddit comment thread
2. you're redirected to /void(0)

Screenshots

[TheFrenchGhosty]

void(0) is used to trigger some JS.

For example to open the Reddit comments: you click on "View Reddit comments" that link to "javascript:void(0)"

I'm guessing it's a mistake someone made in the code and instead of "javascript:" it's the instance URL.

[TheFrenchGhosty]

The problem is that Invidious is serving:

<a href="void(0)" data-onclick="toggle_parent">[ - ]</a>

Even though the line in the code seems correct:

https://github.com/iv-org/invidious/blob/master/src/invidious/comments.cr#L474

Edit: found the issue:

https://github.com/iv-org/invidious/blob/master/src/invidious/comments.cr#L539

[TheFrenchGhosty]

A 10$ bounty has been added to this issue.

Anyone opening a PR fixing this issue, will receive 10$ (in BTC) from the Invidious project.

More details: #1898

[fristys]

Can confirm this is still an issue. Any progress on it?

Also, instead of relying on javascript:void(), wouldn't be more sensible to actually use event.preventDefault() somewhere here

invidious/assets/js/handlers.js

Line 16 in e2fc642

e = e.target;

? Or maybe add an extra data- attribute to specify whether the event should be prevented or bubbling should be stopped and then use that in the handlers file?

[TheFrenchGhosty]

@fristys

Any progress on it?

I don't think so.

Also, instead of relying on javascript:void(), wouldn't be more sensible to actually use event.preventDefault() somewhere here

Maybe? The JS hasn't really ever been changed since no one in the team is a JavaScript developer.

[fristys]

Also, instead of relying on javascript:void(), wouldn't be more sensible to actually use event.preventDefault() somewhere here

Maybe? The JS hasn't really ever been changed since no one in the team is a JavaScript developer.

Normally, I'd offer my help, but, sadly, at this time my day job doesn't give me a lot of spare time.

I think it's a good practice to use preventDefault / stopImmediatePropagation instead of voids (it'll also save you having to write the whole url thing). It gives you more control over event handling, and a guarantee that native browser events don't get triggered when you don't want them to.

[SamantazFox]

@fristys yes, in the future I'd like to rewrite all links with data-* attributes and register event handlers in JS.
And as ghosty said, there aren't JS devs in the team, so PRs are much welcome :)