Skip to content

Conversation

@Mamaduka
Copy link
Member

@Mamaduka Mamaduka commented Jul 5, 2022

What?

Resolves #21699.
Alternative to #30100.

PR tries to use https as the default protocol if the URL doesn't appear to contain schema and it's not a relative link starting with //.

Why?

If URL has no schema esc_url, prepend it with http://. All social media platforms use https://, so it makes sense to be the default fallback.

How?

I've used slightly modified logic from esc_url. Schema is checked using parse_url and code only checks for relative links with // prefix.

Testing Instructions

  1. Open a Post or Page
  2. Insert the example block (snippet below).
  3. Save and view the post.
  4. On the front-end inspect block elements and confirm that only WordPress social link has a default protocol.

Snippet

<!-- wp:social-links -->
<ul class="wp-block-social-links"><!-- wp:social-link {"url":"//twitter.com/WordPress","service":"twitter"} /-->

<!-- wp:social-link {"url":"profiles.wordpress.org/wordpress/","service":"wordpress"} /-->

<!-- wp:social-link {"url":"http://github.com/WordPress","service":"github"} /--></ul>
<!-- /wp:social-links -->

Screenshots or screencast

@Mamaduka Mamaduka requested a review from ajitbohra as a code owner July 5, 2022 18:38
@Mamaduka Mamaduka requested review from aristath and ndiego July 5, 2022 18:38
@Mamaduka Mamaduka self-assigned this Jul 5, 2022
@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Block] Social Affects the Social Block - used to display Social Media accounts labels Jul 5, 2022
Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@Mamaduka
Copy link
Member Author

Mamaduka commented Jul 6, 2022

Thanks for the review, Ari ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Social Affects the Social Block - used to display Social Media accounts Needs User Documentation Needs new user documentation [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Social Icon URLs default with incorrect protocol

4 participants