Support from Acquia helps fund testing for Drupal Acquia logo

Comments

RobLoach’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Active » Postponed

http://groups.google.com/group/disqus-dev/search?group=disqus-dev&q=http...

You could use http://drupal.org/project/securepages to redirect from https to http so that Disqus works. Other then that, let me know if you get it working.

BeWhy’s picture

We are experiencing a problem with the difference betweeen http and https. We submit comments successfully (as evidenced at the bottom of the node and in disqus). However, we can only view correct comment counts when viewed from a view using https, the comment count reads "0" when viewed from an http page.

we are using version 6.x-1.9

This behavior is the same whichever setting we use with secure pages -whether we force using secure pages or not. We'd rather not resort to forcing ssl.

Apparently wo****ess :) got an update and some sort of explanation here of what the fix was here: http://stackoverflow.com/questions/9780140/how-to-enable-ssl-support-on-.... Some investigation into that plugin reveals that it has a function to detect whether the connection is secure or not and it re-writes the connection type according to http/https...

geerlingguy’s picture

Status: Postponed » Needs review
FileSize
1.68 KB

Attached patch adds SSL support by simply passing http/https depending on the current request. Works for me! (Disqus seems to have enabled SSL support sometime in June 2011...)

geerlingguy’s picture

FileSize
2.5 KB

Whoops! Forgot the teaser/count. Now the protocol is passed for that as well.

alias.mac’s picture

You have a php error in you patch when closing the array - 2 parenthesis (see the new patch bellow with the fix).
Also there is a problem loading Disqus on a mix HTTP/HTTPS and if we want to manually force https we should use the ?https query parameter.
More information found here:
http://help.disqus.com/customer/portal/articles/542119-can-disqus-be-loa...

Therefore, I'm not quite sure, but if the web site should load in a mix mode (serving both HTTP and HTTPS requests) then we should probably pass the ?https query parameter always.
I'll be making some tests during this weekend and will post later what I found.

pcavanaugh’s picture

Here is a new patch that adds the "?https" parameter. I also added a Force SSL option in the config panel - I needed this because my site is behind a proxy and doesn't have $_SERVER['HTTPS'] set even though the site is served over SSL.

chasingmaxwell’s picture

I recently hacked the disqus.js file to make it work over a secure connection (before I knew about this issue and the patches here). I would be very interested to know if this will be included in the next release.

geerlingguy’s picture

A quick note, too: Disqus changed its feature set sometime this summer so that, even if you use this module patch, some Disqus resources may still be loaded over the non-SSL Disqus CDN. See: http://stackoverflow.com/a/10015089/100134

geerlingguy’s picture

Status: Needs review » Reviewed & tested by the community

But the patch as it stands looks and works great, and I like pcavanaugh's addition of a 'force SSL' setting, too.

alias.mac’s picture

I had some problems with the disqus SSL support, to have it always load on https (because I serve the website on both http and https).
The patch worked great, but it took so long to update on the disqus side, that I thought it didn't. So, I think it's best to give that information (help) on the form that chasingmaxwell made on the patch ($form['behavior']['disqus_force_ssl']).

urlaub’s picture

In addition to the patch I had to change all http://disqus.com links to //disqus.com in the file disqus.module to get the widgets working without breaking ssl!
Currently you can't use the 2012 features, They have to deactivated! Actually the fresh 2012 design was kind of what had drawn my attention to disqus....

<div id="dsq-recentcomments" class="dsq-widget"><script type="text/javascript" src="//disqus.com/forums/$domain/recent_comments_widget.js?num_items=$num_items&amp;excerpt_length=$excerpt_length$avatars"></script></div>
EOT;
    break;
    case 'disqus_popular_threads':
      $subject = t('Popular Threads');
      $content = <<<EOT
<div id="dsq-popthreads" class="dsq-widget"><script type="text/javascript" src="//disqus.com/forums/$domain/popular_threads_widget.js?num_items=$num_items"></script></div>
EOT;
    break;
    case 'disqus_top_commenters':
      $subject = t('Top Commenters');
      $content = <<<EOT
<div id="dsq-topcommenters" class="dsq-widget"><script type="text/javascript" src="//disqus.com/forums/$domain/top_commenters_widget.js?hide_mods=$hide_mods&amp;num_items=$num_items$avatars"></script></div>
EOT;
    break;
    case 'disqus_combination_widget':
      $subject = t('Comments');
      $content = <<<EOT
<script type="text/javascript" src="//disqus.com/forums/$domain/combination_widget.js?num_items=$num_items&amp;hide_mods=$hide_mods&amp;excerpt_length=$excerpt_length&amp;color=$color&amp;default_tab=$default_tab"></script>
alias.mac’s picture

That's strange.
What do you mean by breaking the SSL? Warnings on the browser?

urlaub’s picture

with breaking the SSL I mean that the connection is only patially encrypted, because the javascript files loaded by the disqus widgets, are hardcoded with http://

Nevertheless, currently it does break the secure connection with the // edit as well! I tried hardcoding https:// but than the disqus comments block disappeared?

The main problem is that the browser does show a different icon for broken ssl connection, which can confuse the site visitor, check here: https://www.cascada.travel/Travel-Companion-Hub

alias.mac’s picture

Yes, well you can see in my comment above http://drupal.org/comment/reply/969202/6558580#comment-6487448
that the problem is on the Disqus service and not from the Drupal side. The JS requests are asking for the https files and the Disqus is serving you with cached ones (not HTTPS). You'll just have to wait a few days until it's fixed or contact Disqus to ask them to fix that :)

The patch provided before by pcavanaugh should probably work fine (I didn't tested yet, but if it works like the code looks like, it's fine).

modctek’s picture

Has #5 been committed yet?

Patching with #5 now gives an error, saying there is an unexpected comma at line 272 in disqus.module. I suspect there is a missing comma somewhere in the patch.

Nevermind, saw the correction. >_<

Will either of the patches be committed?

alias.mac’s picture

I don't know when it will be committed, but there is also a request for 6.x version: http://drupal.org/node/1904598. I think it can be addressed easily on both versions.

geerlingguy’s picture

Any chance of getting this committed? Just ran into this bug on another site today :)

nadavoid’s picture

I made a small update to #5, using the global boolean $is_https. Drupal provides this and considers several server environment variables before determining whether the request was via http or https. This should more consistently properly detect wether to use https.

pianomansam’s picture

I just had to patch this one two different sites today. Can we PLEASE get committed?

RobLoach’s picture

What if we were to use "//" as the protocol instead of checking for https/http? "//" tells the brower to use whatever protocol is currently in use.

geerlingguy’s picture

Status: Reviewed & tested by the community » Needs work

A good reference for the workability of // is this answer (and comments) on Stack Overflow. The only major downside/caveat has to do with rendering things outside a browser:

One downside occurs if your URLs are viewed outside the context of a web page. For example, an email message sitting in an email client (say, Outlook) effectively has no URL, and when you're viewing a message containing a protocol-relative URL, there is no obvious protocol context at all (the message itself is independent of the protocol used to fetch it, whether it's POP3, IMAP, Exchange, uucp or whatever) so the URL has no protocol to be relative to.
...
Similar problems could occur in other non-HTTP contexts such as in tweets, SMS messages, Word documents etc.

Since Disqus should really only appear in the context of a full page load/render inside a browser environment, I think it would work okay.

geerlingguy’s picture

Status: Needs work » Needs review
FileSize
814 bytes

Attached patch switches to //. I'm going to test this on a few SSL sites I have in a variety of browsers, and I'll see if there's any problem with this approach.

[Edit: Browser testing results below... I tested on a page with https and with http]

  • Chrome (Mac): works
  • FireFox (Mac): works
  • Safari (Mac): works
  • Safari (iOS): works
  • Android (4.x): works
  • Internet Explorer: ?

I've applied this patch on this page on Server Check.in, which uses HTTPS, and it seems to work fine. If you have IE7/8/9/10 available, could you make sure it's working there, too?

dashohoxha’s picture

FileSize
10.95 KB

I don't know why, but the above patch (on #22) didn't work for me. The blocks of Recent Comments etc. didn't display. Google Chrome prevents retrieving HTTP data from inside an HTTPS page.

Instead, the attached patch did work. It is a simple replacement of everything 'http://' with 'https://'.

artofeclipse’s picture

I managed to get it working just by remove the http protocol from the js file (disqus.js)

/* Line 54*/
...
-    url: 'http://' + disqus_shortname + '.disqus.com/embed.js',
+   url: '//' + disqus_shortname + '.disqus.com/embed.js', 
...

and

/* Line 66*/
...
-    url: 'http//' + disqus_shortname + '.disqus.com/count.js',
+   url: '//' + disqus_shortname + '.disqus.com/count.js',
...

doing this will make the browser use the protocol of the parent page, not sure if this is a good approach to do this.

figureone’s picture

artofeclipse is correct in #24, the module should be using the "protocol relative URL" instead of specifying "http:".

Fixing those two URLs in disqus.js will let the module work on https-enabled Drupal sites.

Source:
http://paulirish.com/2010/the-protocol-relative-url/

AaronBauman’s picture

Patch below

AaronBauman’s picture

Title: SSL/Https Support » Respect protocol of the page request
Category: support » bug
FileSize
4.25 KB

Serving page results with mixed assets in http/https causes disqus to be blocked by Chrome (and possibly other browsers).
Attached patch uses the same protocol as the page request to fetch the remote js assets.

slashrsm’s picture

Status: Needs review » Needs work

Needs reroll against latest dev.

geerlingguy’s picture

Status: Needs work » Needs review

Patch in #22 should still apply, and was tested in all browsers except IE so far...

slashrsm’s picture

➜  disqus git:(7.x-1.x) wget -qO- https://drupal.org/files/disqus-https-1159690_0.patch | patch -p1
patching file disqus.js
Hunk #2 succeeded at 66 (offset 14 lines).
Hunk #3 succeeded at 78 (offset 14 lines).
patching file disqus.module
Hunk #1 FAILED at 495.
Hunk #2 succeeded at 339 with fuzz 2 (offset -163 lines).
Hunk #3 FAILED at 510.
2 out of 3 hunks FAILED -- saving rejects to file disqus.module.rej

There was some refactoring code pushed to -dev about a week ago (before you posted a patch), so you are most likely working on a non-updated env.

Thanks for your effort. Looking forward to commit this once ready.

AaronBauman’s picture

rerolled @ dev

added hidden option "disqus_use_https" to conform to $options configuration in disqus_block_view().
this option could be exposed on an admin screen, but not strictly necessary for this issue.

AaronBauman’s picture

Status: Needs review » Needs work

found a js bug in IE8 (grrrr)

geerlingguy’s picture

Status: Needs work » Needs review
FileSize
814 bytes

@aaronbauman - We tried using a similar method to what you're doing in #31 earlier, but it looks like the use of the // protocol will be simpler and work just as well (if not better - see #20 and the following conversation).

The attached patch is a re-roll of #22, updated with the latest dev code...

AaronBauman’s picture

CROSS POST, PLEASE IGNORE:

AaronBauman’s picture

OK, but then we should use the same approach in _disqus_block_content()
(to my surprise, url() handles // correctly)

geerlingguy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me!

alias.mac’s picture

Like it was posted on https://drupal.org/node/969202#comment-7324542, the patch on #22 (https://drupal.org/node/969202#comment-7293692) won't work because disqus caches the page in either http or https (the first request made). So I think it would be better to just have on the administration page a checkbox saying that we want to use https or not. This is better if we want to serve a website in both http and https (mixed requests), since http can have https on the disqus block.
Just my 2 cents.

slashrsm’s picture

Status: Reviewed & tested by the community » Fixed

Commited. Thank you all for your effort!
http://drupalcode.org/project/disqus.git/commit/fdeb85b

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit fdeb85b on 7.x-1.x, 8.x-1.x by slashrsm:
    Issue #969202 by geerlingguy, aaronbauman, alias.mac, pcavanaugh,...