Fix for WP_Dependencies::dequeue

[chrisjean]

The WP_Dependencies::dequeue method has three major flaws that prevent it from working: does not split handle around '?' to divide handle from arg (see the WP_Dependencies::enqueue method), tries to remove the handle from the queue array by using the handle as an index of the array when the handle is a value of the array, and does not remove a relevant entry from the args array if the handle has an arg.

The provided patch (wp-dependencies-patch.txt) was tested with 2.8-beta1-11389 and corrected all three issues.

[Denis-de-Bernardy]

Can you highlight an actual use-case for the behavior? I can't make sense of using a handle with a param in it.

[chrisbliss18]

Frankly, I don't understand that either; however, I'm matching the functionality found in the WP_Dependencies::enqueue method. The existing code for the two methods is as follows:

    function enqueue( $handles ) {
        foreach ( (array) $handles as $handle ) {
            $handle = explode('?', $handle);
            if ( !in_array($handle[0], $this->queue) && isset($this->registered[$handle[0]]) ) {
                $this->queue[] = $handle[0];
                if ( isset($handle[1]) )
                    $this->args[$handle[0]] = $handle[1];
            }
        }
    }

    function dequeue( $handles ) {
        foreach ( (array) $handles as $handle )
            unset( $this->queue[$handle] );
    }

As you can see, the enqueue method does a number of things that the dequeue method does not attempt to duplicate. In addition, the fact that it attempts to improperly remove an item from the array using a value as an index causes a complete failure of the method. Thus, it is extremely easy to create unexpected results. The code I used to test the functionality of the class is as follows:

    $dep = new WP_Dependencies;

    $dep->add( 'test', '' );
    $dep->add( 'another', '' );
    $dep->add( 'this', '' );
    $dep->add( 'that', '' );
    $dep->add( 'the_other', '' );
    $dep->add( 'handle', '' );
    $dep->add( 'not_used', '' );
    $dep->add( 'data_handle', '' );

    $dep->enqueue( array( 'test', 'another' ) );
    $dep->enqueue( array( 'this', 'that' ) );
    $dep->enqueue( 'the_other' );
    $dep->enqueue( 'handle?data' );
    $dep->enqueue( 'data_handle?dh_data' );

    $dep->dequeue( array( 'test', 'another', 'handle?data' ) );

    print_r( $dep->queue );
    print_r( $dep->args );

When run, you can see that the dequeue call fails to remove anything from the queue and args.

While I may not understand all the use cases, it does not change the fact that the dequeue method as it stands now fails to remove what is added by the queue method. My patch resolves the issues and allows the method to properly remove handles and args added by the queue method.

FYI: Not that it matters to me, but it may somewhere internally. This isn't a JavaScript component. The class is used as a parent class for both WP_Styles and WP_Scripts.

[Denis-de-Bernardy]

Re the component, it doesn't matter much -- it's just that Andrews usually looks into tickets related to WP_Dependencies.

[Denis-de-Bernardy]

andrew?

[westi]

Assiging to Andrew for review.

Will need merging with BackPress when changed.

Would be good to write some tests for these classes at some point.

[westi]

Fix typo :-(

[azaozz]

WP_Dependencies::dequeue hasn't been changed since the class was added over a year ago and doesn't seem used anywhere in WordPress, may possibly be used in BackPress.

At the same time it doesn't really dequeue a script as all dependencies are left in the queue. Despite that the patch seems like a good idea, will at least bring it in line with WP_Dependencies::enqueue.

Support for query strings in the handle seems to have been added for back-compat, haven't seen that used anywhere. Will have to coordinate with other apps using BackPress before adding it.

[Denis-de-Bernardy]

punting per IRC discussion. this can wait until 2.9 unless andrew has objections.

[azaozz]

(In [12049]) Bring WP_Dependencies::dequeue() in line with WP_Dependencies::enqueue(), props chrisbliss18, fixes #9870