Embedded Discourse url problems

[tendon]

• This is a question about using the theme.
• This is a feature request.
• I have updated all gems with bundle update.
• I have tested locally with bundle exec jekyll build.
• I believe this to be a bug with the theme --- not Jekyll, GitHub Pages or one of the bundled plugins.

---

Environment informations

• Minimal Mistakes version: 4.5.0

---

Expected behavior

Integration with discourse as a comments provider doesn't work in all conditions.

Steps to reproduce the behavior

1. Set up _config.yml appropriately for discourse embedding.
2. Follow the instructions at https://meta.discourse.org/t/embedding-discourse-comments-via-javascript/31963. In particular notice the code listed in step 4, and compare it to the code generated by _includes/comments-providers/discourse.html.

There are two problems.

First, discourseUrl will be given an extra pair of forward slashes:

discourseUrl: '//http://discourse.example.com/'

while it should be:

discourseUrl: 'http://discourse.example.com/'

Second, the canonical url is missing (at least) the site.url and appears as:

discourseEmbedUrl: '/blog/entry-123.html'

while it should be

discourseEmbedUrl: 'http://example.com/blog/entry-123.html'

A fix is proposed in this PR: #1181

Note: in order for everything to work correctly, you must ensure that _config.yml contains the proper settings (site.url and site.baseurl) for the urls!

[mmistakes]

I believe the thinking behind discoureUrl was to feed it a protocol-less URL. SO instead of:

comments:
  provider: "discourse"
  discourse:
    server: "http://discourse.example.com/"

You give it this:

comments:
  provider: "discourse"
  discourse:
    server: "discourse.example.com"

Which is probably more of a documentation issue than an actual bug.

[tendon]

But does it work if protocol-less? Would the protocol be added somehow?
(All examples I've seen - especially at https://meta.discourse.org/t/embedding-discourse-comments-via-javascript/31963 - always have the protocol in the generated JS.)

[tendon]

Before my patch it didn't work. What prompted the solution was this: https://meta.discourse.org/t/solved-error-embedding/57276/2.

Note the statement: "Make sure the discourseEmbedUrl is a full URL, not a relative path"

[mmistakes]

@tendon Smells like an issue with your environment or configuration. If you are triggering the correct Jekyll production environment that path should be absolute. Your PR just swapped the absolute_url filter with site.url and site.baseurl prepends which should be the same.

If they're not and you're getting relative URLs then it's a bug with Jekyll... or perhaps you're on an older version that needs updating.

[mmistakes]

@tendon People like to use protocol less URLs so they can swap between both https:// and http:// (secure/insecure)... usually when building for dev/prod environments.

As far as the browser is concerned http://yourdomain.com and //yourdomain.com are the same. You'll see protcol-less URLs a lot in <script> embeds for things like jQuery on CDNs and the like.

[tendon]

I'll check that about the production environment. Thanks for the hint.

But doesn't //yorudomain.com try to open with the file protocol (file:///yourdomain.com)?

[mmistakes]

Nope, it shouldn't.

Reference: https://www.paulirish.com/2010/the-protocol-relative-url/

[mmistakes]

But it will if you're just opening local .html files in a browser. When opened from a file server (which is where they will eventually live) it's not an issue.

When in doubt fire up Jekyll's server with jekyll serve and test your site that way. Opening the pages directly without the WEBbrick server that Jekyll spins up is probably why you're having problems.

[tendon]

Yes it seems ENV production or development has something to do with it; and I was checking one while I should have been looking at the other one. One possible improvement is to include the embedding only in production. Please see https://joebuhlig.com/discourse-commenting-for-jekyll/, the section "Development Tweaks".

Still trying to see if protocol-less urls work, though.

[tendon]

Yes, the protocol-less urls seem to work too. So this should be documented better.

However, I would suggest a better way is to take away the // from the assignment of discourseUrl because then it should work in all cases:

• People who don't know about the protocol-less trick (like me before this conversation!) will include the protocol in their site.comments.discourse.server setting.
• People who know and want to use the trick can set it explicitly there, i.e. by assigning //discourse.yourdomain.com to that same setting.

WDYT?

[mmistakes]

My hesitation to removing the // is it's not backwards compatible and will break sites that don't specify a protocol for site.comments.discourse.server... eg. meta.discourse.org

Adding a note to the documentation that make it clearer that you shouldn't add a protocol to comments.discourse.server in _config.yml is the way to go for sure.

What I would suggest is adding some logic to the capture and test for :// in the variable. If it's there leave it alone... if not then prepend: '//'. I do something similar with images to test if they're relatively hosted or externally.

There could be some edge cases that doesn't catch so not sure if that's the best way forward either. My guess is Discourse is so rarely used with the theme it's not a big issue. Though I'd feel better if those who actually use it chime in, if the current implication needs improvement/is broken.

[tendon]

OK. I see the point of backward compatibility.
So you can ignore the PR, but maybe do update the documentation for future users!
Thanks.

[mmistakes]

Will do. Thanks for taking the time to start the discussion. Always good to get feedback on parts of the theme that might not be working as expected.