We should use protocol relative urls to load external resources to avoid browser messages about unsafe objects on the page if someone accesses the site using https.
The attached patch removes the http: from all resources where I thought it's applicable.
I've tested this only with the google js - not with the other adjustments.

The patch also contains removed trailing white-spaces but I think it's a manageable amount ;)

CommentFileSizeAuthor
#11 openlayers-osm-protocol-relative-fix-1458930-11.patch540 bytestimwood
#5 openlayers-protocol-relative-fix-1458930-5.patch3.56 KBcoredumperror
#2 openlayers-use-protocol-relative-urls-1458930-2.patch7.07 KBdas-peter
openlayers-use-protocol-relative-urls.patch11.21 KBdas-peter
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, openlayers-use-protocol-relative-urls.patch, failed testing.

das-peter’s picture

das-peter CreditAttribution: das-peter commented
Status: Needs work » Needs review
FileSize
openlayers-use-protocol-relative-urls-1458930-2.patch7.07 KB

I totally failed creating the patch :| Next try :D

zzolo’s picture

zzolo CreditAttribution: zzolo commented
Status: Needs review » Fixed

Hey @das-peter. Thanks for the patch! I think this is a great change. I have committed the change.

http://drupalcode.org/project/openlayers.git/commit/f0ab3a6

Status: Fixed » Closed (fixed)

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

coredumperror’s picture

coredumperror CreditAttribution: coredumperror commented
Priority: Minor » Critical
Status: Closed (fixed) » Needs work
FileSize
openlayers-protocol-relative-fix-1458930-5.patch3.56 KB

This change completely broke openstreetmap- and mqcdn-based tiles on sites which use https for logged in users. *.mqcdn.com and *.openstreetmap.org don't offer their tiles through https:// at all, making it totally impossible for users logged into an SSL drupal site to input map data, since they can't see the tiles.

I'm not entirely certain that this is actually necessary for google, either, but at least it doesn't (appear to) break anything.

I've attached a patch that puts the http:// back onto all URLs for openstreetmap and mqcdn, as well as the one link to labs.metacarta.com, since it also doesn't serve that file through https.

coredumperror’s picture

coredumperror CreditAttribution: coredumperror commented
Status: Needs work » Needs review

Whoops, I set the wrong status.

Pol’s picture

Pol
the/that
Location Brussels
CreditAttribution: Pol commented
Status: Needs review » Needs work

Hello,

Could you reroll your patch against the latest dev version please ?

Thanks !

Pol’s picture

Pol
the/that
Location Brussels
CreditAttribution: Pol commented
Status: Needs work » Fixed

I did it and committed.

coredumperror’s picture

coredumperror CreditAttribution: coredumperror commented

Sorry for the lack of response, Pol. I was on vacation at the time. Thank you for rerolling and committing my patch.

Status: Fixed » Closed (fixed)

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

timwood’s picture

timwood
Primary language English
Location Rockville, Maryland
CreditAttribution: timwood commented
Status: Closed (fixed) » Needs work
FileSize
openlayers-osm-protocol-relative-fix-1458930-11.patch540 bytes

It looks like the fix for openstreetmap.org in #5 above was lost and never actually committed or something. I cannot find the change from the patch in any release or commit since Pol posted comment #8 above on 12/30/2012. The patch from das-peter committed by zzolo above in comments #2 and #3 above was the last time the osm base url line of code was changed, in that case it was changed to be protocol agnostic, which does not work as osm does not serve up tiles via HTTPS.

Attached patch reverts the OSM base url line back to using http://.

Also, I've flagged http://drupal.org/node/1947770 as a duplicate of this.

timwood’s picture

timwood
Primary language English
Location Rockville, Maryland
CreditAttribution: timwood commented
Status: Needs work » Needs review

Sorry, should have been needs review.

Pol’s picture

Pol
the/that
Location Brussels
CreditAttribution: Pol commented
Status: Needs review » Fixed

Thanks TimWood,

Committed on 7.x-2.x and 7.x-3.x.

Status: Fixed » Closed (fixed)

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