search

osompress / simple-social-icons

Plugin: Simple Social Icons
62 stars 33 forks source link

Use esc_attr instead of esc_url because of % sign in URL #108

Closed johnstonphilip closed 4 years ago

johnstonphilip commented 4 years ago

Fixes #107

How to test

To Reproduce Steps to reproduce the behavior:

  1. Add the Social Icons widget to your sidebar.
  2. Add some social links.
  3. View the page.
  4. See the warning.

Warning: sprintf(): Too few arguments in simple-social-icons.php on line 458

To test the fix Steps to test the fix:

  1. Switch to branch fix/sprintf-issue
  2. View the page from the steps above.
  3. See icons/links now show properly.

Documentation

No documentation required.

Suggested changelog entry

johnstonphilip commented 4 years ago

Before:

Screen Shot 2020-04-02 at 4 40 19 PM

After:

Screen Shot 2020-04-02 at 5 15 55 PM
nickcernis commented 4 years ago

@johnstonphilip I'm struggling to reproduce the original error at the moment. 🤷‍♂

So far I don't see, "Too few arguments in simple-social-icons.php on line 458" by default with the current develop branch using the repro steps above (PHP 7.4.3, WP 5.4). Where do you see % in the URL? Do your social icons widget URIs include special characters of some kind?

Your fix looks good. I'm just curious to learn why we haven't seen this until now.

dreamwhisper commented 4 years ago

I'm struggling to reproduce the original error at the moment.

I'm also struggling to reproduce this.

johnstonphilip commented 4 years ago

Oh I found it. It's because I have a space in my plugin name, due to accidentally cloning the plugin twice

http://php5.local/wp-content/plugins/simple-social-icons 2/symbol-defs.svg#social-behance

That space is getting converted to a % sign, and thus triggering sprintf