Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore anchors when using "Issue title contains page URL" mode5 #55

Closed
phil-opp opened this issue Aug 17, 2018 · 3 comments · Fixed by #59
Closed

Ignore anchors when using "Issue title contains page URL" mode5 #55

phil-opp opened this issue Aug 17, 2018 · 3 comments · Fixed by #59
Labels

Comments

@phil-opp
Copy link
Contributor

I use autogenerated anchors for every heading on my site for a table of contents. When clicking such an anchor link the URL changes to example.com#anchor-title. When I now refresh the page (i.e hit F5), the comment section stays empty because utterances looks for an issue title with the correct #anchor-title. If I now post a comment a new issue is created for the same page.

To fix this, I would propose to just ignore any anchors when doing URL matching, or at least providing an option to do so. What do you think?

@jdanyow
Copy link
Member

jdanyow commented Aug 17, 2018

I agree, I think this is a bug. We should ignore both querystring/search and hash/anchor by default. Also need to normalize trailing backslash.

For example, this url in the address bar:

https://os.phil-opp.com/minimal-rust-kernel/?foo=bar&b#anchor

Should result in a search for issues whose title contains:

https://os.phil-opp.com/minimal-rust-kernel

Thoughts?

@jdanyow jdanyow added the bug label Aug 17, 2018
@phil-opp
Copy link
Contributor Author

Sounds good to me! However, I think there are some sites that use the query string for determining which article to show (e.g. ?article_id=1234).

Maybe we can use the rel=canonical link when present and default to ignoring query string/anchor else?

@phil-opp
Copy link
Contributor Author

phil-opp commented Oct 9, 2018

I know that I'm a bit late, but thanks for fixing this so quickly! It works without problems now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants