Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor fix to prevent a potential undefined variable notice. #184

Merged
merged 2 commits into from
Sep 25, 2015

Conversation

jrfnl
Copy link

@jrfnl jrfnl commented Sep 24, 2015

(and slight improvement to the regex)

Fixes:
Undefined offset 1 in /path/to/wp-content/plugins/ricg-responsive-images/wp-tevko-responsive-images.php:350

@jrfnl
Copy link
Author

jrfnl commented Sep 25, 2015

Hmm... is it just me or are you not using the WP Coding Standards ? I looked at why linthub is failing and all I see are things which are in direct contrast to the WPCS.

@joemcgill
Copy link
Member

Good catch @jrfnl. We could probably cut it down even further and do something like:

if ( preg_match( '/ width="([0-9]+)"/', $atts, $width ) &&  preg_match( '/ height="([0-9]+)"/', $atts, $height ) ) {
  $size = array(
    (int) $width[1],
    (int) $height[1]
  );
}

Spaces before the attributes are probably a good idea. And, yes, our Linthub config is acting up right now so you can ignore that check for now (see: #185).

@jrfnl
Copy link
Author

jrfnl commented Sep 25, 2015

You're welcome ;-)

joemcgill added a commit that referenced this pull request Sep 25, 2015
Sanity check the `preg_match()` values for width/height before using.

Props @jrfnl
@joemcgill joemcgill merged commit 20301c7 into ResponsiveImagesCG:dev Sep 25, 2015
@jrfnl jrfnl deleted the feature/fix-undefined-notice branch January 24, 2016 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants