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

Tag nojs page doesn't use discussion slugger #3013

Closed
clarkwinkelmann opened this issue Aug 11, 2021 · 3 comments · Fixed by flarum/tags#142
Closed

Tag nojs page doesn't use discussion slugger #3013

clarkwinkelmann opened this issue Aug 11, 2021 · 3 comments · Fixed by flarum/tags#142
Assignees
Labels
Milestone

Comments

@clarkwinkelmann
Copy link
Member

clarkwinkelmann commented Aug 11, 2021

Bug Report

Current Behavior
The nojs PHP view for tags doesn't use the discussion slugger introduced in #2456

With the default driver selected, this causes the ID to be repeated twice and therefore the wrong canonical URL to be used. With a custom slug driver, URLs would be broken.

Steps to Reproduce

  1. Enable tag extension
  2. Create discussion in tag
  3. View source code of <flarum url>/t/<tag slug>
  4. Observe wrong links being rendered in the <noscript> section

Expected Behavior
The links should be rendered identically to Flarum core's index nojs content.

Core nojs which has been updated to work with the slugger:
https://github.com/flarum/core/blob/f1ba5e7b7081767a6bfcef4a2736728cf35b9b0e/views/frontend/content/index.blade.php#L9-L13

Tag nojs which hasn't been updated:
https://github.com/flarum/tags/blob/7c9a3ca8d7de5c9d4b6aa7f0275f4b6f2ec20667/views/frontend/content/tag.blade.php#L10-L14

Screenshots
Screenshot of https://discuss.flarum.org/t/extensions source HTML
image

Environment

  • Flarum version: 1.0.4

Possible Solution
https://github.com/flarum/tags/blob/master/views/frontend/content/tag.blade.php needs to be updated to work like https://github.com/flarum/core/blob/master/views/frontend/content/index.blade.php

Additional Context
Reported here https://discuss.flarum.org/d/28599-tag-pages-seo-issues

@ayyilmaz
Copy link

https://discuss.flarum.org/d/27915-goog-search-console-excluded-pages

The excluded links have a URL structure that I don't understand on Google Search Console. What could be the reason for this? Currently discussion id is repeated.

image

@davwheat davwheat added this to the 1.1 milestone Aug 11, 2021
@clarkwinkelmann
Copy link
Member Author

@ayyilmaz yes your issue is very likely coming from this as well.

Google will likely index these incorrect URLs, and then find out that it's a duplicate URL when it later visits the page and sees the meta canonical tag on the page.

@askvortsov1
Copy link
Sponsor Member

This is tangential, but I don't like that we duplicate "discussion list" code between core and tags. Could we use an "included" template for both? IIRC this is possible with blade. We might also want to look into overridability of cores blade templates.

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.

5 participants