Code Monkey home page Code Monkey logo

Comments (11)

tollmanz avatar tollmanz commented on September 12, 2024

Hey @vidluther! I just ran into this same issue. The regex for the replacement always replaces the URL with "http://". I think the easiest solution is to just remove the "http://" and replace it with "//", which will produce protocol aware URLs.

I am interested in why @markjaquith made this work for "http" only. I'm guessing that SSL was not required for the project in which this originated and thus it was never addressed. The patch should be as easy as deleting the 5 characters.

from wp-stack.

joshlyford avatar joshlyford commented on September 12, 2024

@vidluther we use an is_ssl check and change the output an see it on the gist below. Not sure its the best fix but it works well for us.

https://gist.github.com/joshlyford/8700675

from wp-stack.

vidluther avatar vidluther commented on September 12, 2024

Thanks @joshlyford , @tollmanz does that work for you as well?

from wp-stack.

tollmanz avatar tollmanz commented on September 12, 2024

The is_ssl() approach will definitely work; however, there are some potential pitfalls. If your page caching solution does not take into account the page protocol when caching the page (i.e., treats https://mysite.com and http://mysite.com as the same), you will end up with mixed content warnings if the first person to visit your page is using http and the second user uses https. I recently learned that batcache has issues with this.

My solution would use a protocol relative URL, which allows you to use the same URL regardless of whether the current page is http or https. You can take a look at my diff.

from wp-stack.

markjaquith avatar markjaquith commented on September 12, 2024

Confirmed that the project I originally wrote it for did not require SSL. I like the protocol-relative trick. If you're serving up SSL pages, you'll need an SSL-capable CDN to avoid mixed content warnings, so I'm okay with this assumption.

from wp-stack.

phikai avatar phikai commented on September 12, 2024

@markjaquith any chance we can re-open this for discussion?

I think the fix that @joshlyford put forward by using is_ssl() may be a better solution... although as @tollmanz points out there may be problems.

After having used the protocol relative fix in a "wide" scale for a few weeks, there are quite a few issues with some third party tools that use content from the sites:

  • Facebook Open Graph doesn't allow them
  • Some RSS Feed Readers don't support them
  • Some email/newsletter tools don't support them
  • Google News doesn't support them

Granted, the URLs are RFC compliant (http://tools.ietf.org/html/rfc3986), so it's a tough call.

from wp-stack.

krogsgard avatar krogsgard commented on September 12, 2024

@markjaquith +1 to @phikai's notes.

We're struggling w/ the relative protocol URLs in Mailchimp RSS campaigns, as well as Open Graph Facebook images. I'm all for the new standard, but even though the browser support is good, it seems many applications aren't there yet.

from wp-stack.

tollmanz avatar tollmanz commented on September 12, 2024

This is a bummer :( It's weird that those services won't respect those URLs.

although as @tollmanz points out there may be problems.

Just note that the issues I mentioned are fairly easily mitigated if your caching solution is aware of SSL vs. non-SSL requests. There are a host of issue you'll likely face if it is not, so it's probably better to patch that first.

from wp-stack.

phikai avatar phikai commented on September 12, 2024

Our caching solution is batcache... which means we're waiting on your PR there (Automattic/batcache#25).

from wp-stack.

tollmanz avatar tollmanz commented on September 12, 2024

@phikai I think that the issue has been fixed with Automattic/batcache@484c7b9. @Viper007Bond dumped what looks to be a few years of code into that commit, which syncs up what they are running on WordPress.com with the released version. I have not updated to that version, so cannot speak to whether or not it fixes the issue. Worse case scenario, you can simply patch Batcache with my PR and you should be set!

from wp-stack.

Viper007Bond avatar Viper007Bond commented on September 12, 2024

Yeah, the version that was here on GitHub was 4 or 5 years out of date. I dumped what we're currently running on WP.com onto GitHub in that commit.

I can't speak to whether that issue is fixed or not. Closer examination would be required.

from wp-stack.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.