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

BUG: Array to String conversion when using Requirements::combine_files with path/type arguments #2501

Closed
nicole-ashley opened this issue Oct 6, 2013 · 1 comment

Comments

@nicole-ashley
Copy link
Contributor

When using Requirements with files whose type needs to be specified manually (eg, their extension is not 'js' or 'css'), the combine_files duplicate check throws a notice because array_intersect casts the array values to string.

Requirements::combine_files('out.min.js', array(
    array('path' => 'themes/mytheme/js/library.js.inc', 'type' => 'js')
);

The example is arbitrary; going by the above I could just rename the file, but in my real-world case I'm dealing with scripts in other languages which are compiled on load. These scripts have suffixes which shouldn't be changed.

// Requirements_Backend::combine_files
public function combine_files($combinedFileName, $files, $media = null) {
    // duplicate check
    foreach($this->combine_files as $_combinedFileName => $_files) {
        $duplicates = array_intersect($_files, $files); // -- HERE --
        if($duplicates && $combinedFileName != $_combinedFileName) {
            user_error("Requirements_Backend::combine_files(): Already included files " . implode(',', $duplicates)
                . " in combined file '{$_combinedFileName}'", E_USER_NOTICE);
            return false;
        }
    }
    ...
}

As array_intersect casts to string, it's casting the input array (and potentially any of these arrays that wind up in combine_files in future checks), causing the warning to be thrown and the comparison fail.

@simonwelsh simonwelsh added the 3.1 label Mar 15, 2014
@simonwelsh simonwelsh self-assigned this Mar 15, 2014
@simonwelsh simonwelsh removed their assignment Jul 2, 2014
@tractorcow
Copy link
Contributor

The parseCombinedFile() method addresses this issue.

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

No branches or pull requests

3 participants