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

Widget visibility: Add support for old-style single widgets #21

Merged
merged 1 commit into from
Jan 15, 2014

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Dec 19, 2013

Props @westonruter

Fixes the following notice:
Undefined offset: 1 at /jetpack/modules/widget-visibility/widget-conditions.php line 251

This is the line that is erroring
list( $basename, $suffix ) = explode( "-", $widget_id, 2 );
The problem is that it assumes the a -number on the end of the widget name

This is not safe as you can widget names like:
icl_lang_sel_widget
recent-comments-2

For more details, view the original ticket:
http://plugins.trac.wordpress.org/ticket/1979

@georgestephanis
Copy link
Member

@cfinke -- thoughts?

@blobaugh
Copy link
Contributor

@jeherve do you have access to WPML or know of another plugin/widget that caused that notice to test against?

@jeherve
Copy link
Member Author

jeherve commented Jan 15, 2014

I reproduced the issue by creating an old single widget, like so:

add_action("widgets_init", array('Widget_name', 'register'));
register_activation_hook( __FILE__, array('Widget_name', 'activate'));
register_deactivation_hook( __FILE__, array('Widget_name', 'deactivate'));
class Widget_name {
  function activate(){
    $data = array( 'option1' => 'Default value' ,'option2' => 55);
    if ( ! get_option('widget_name')){
      add_option('widget_name' , $data);
    } else {
      update_option('widget_name' , $data);
    }
  }
  function deactivate(){
    delete_option('widget_name');
  }
    function control(){
        $data = get_option('widget_name');
        ?>
        <p><label>Option 1<input name="widget_name_option1"
    type="text" value="<?php echo $data['option1']; ?>" /></label></p>
        <p><label>Option 2<input name="widget_name_option2"
    type="text" value="<?php echo $data['option2']; ?>" /></label></p>
        <?php
         if (isset($_POST['widget_name_option1'])){
            $data['option1'] = attribute_escape($_POST['widget_name_option1']);
            $data['option2'] = attribute_escape($_POST['widget_name_option2']);
            update_option('widget_name', $data);
        }
    }
  function widget($args){
    echo $args['before_widget'];
    echo $args['before_title'] . 'Your widget title' . $args['after_title'];
    echo 'I am your widget';
    echo $args['after_widget'];
  }
  function register(){
        wp_register_sidebar_widget( 'my_widget_id', 'Widget name', array('Widget_name', 'widget'));
    wp_register_widget_control( 'my_widget_id', 'Widget name', array('Widget_name', 'control'));
  }
}

@ghost ghost assigned lezama Jan 15, 2014
lezama added a commit that referenced this pull request Jan 15, 2014
Widget visibility: Add support for old-style single widgets
@lezama lezama merged commit 6656bd3 into Automattic:master Jan 15, 2014
@jeherve jeherve deleted the 1979-trac branch January 15, 2014 15:08
@mjangda
Copy link
Member

mjangda commented Jan 22, 2014

FYI, This breaks widgets that have no settings. Working off the example widget above:

add_action("widgets_init", array('Widget_name', 'register'));

class Widget_name {
 function widget($args){
    echo $args['before_widget'];
    echo $args['before_title'] . 'Your widget title' . $args['after_title'];
    echo 'I am your widget';
    echo $args['after_widget'];
  }
  function register(){
       wp_register_sidebar_widget( 'my_widget_id', 'Widget name', array('Widget_name', 'widget'));
  }
}

The widget doesn't render when added to a sidebar.

@georgestephanis
Copy link
Member

I'll revert.

@mjangda
Copy link
Member

mjangda commented Jan 22, 2014

Simple fix:

Index: widget-conditions.php
===================================================================
--- widget-conditions.php       (revision 89338)
+++ widget-conditions.php       (working copy)
@@ -275,7 +275,7 @@
                                }

                                // Old single widget
-                               else if ( false === self::filter_widget( $settings[$id_base] ) ) {
+                               else if ( ! empty( $settings[ $id_base ] ) && false === self::filter_widget( $settings[$id_base] ) ) {
                                        unset( $widget_areas[$widget_area][$position] );

georgestephanis added a commit that referenced this pull request Jan 22, 2014
Previous changeset breaks widgets that have no settings.
Props @mjangda
See: #21
kraftbj pushed a commit that referenced this pull request Jan 12, 2021
Avoid errors when the plugin is installed manually.
tbradsha pushed a commit that referenced this pull request Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Widget Visibility [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants