Problem/Motivation

A external Javascript added to the list of scripts is not blocked. This is because in hook_js_alter

        $parts = explode(':', $script);
        $script = end($parts);

converts https://example.com to /example.com and the script is therefore not unset.

Steps to reproduce

Proposed resolution

Exclude external scripts from the spilt:

      if (strpos($script, 'http') !== 0) {
        $parts = explode(':', $script);
        $script = end($parts);
      }

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#10 eu_cookie_compliance-blocking-javascripts-not-working-3163235-10.patch1.53 KBsvenryen
#4 eu_cookie_compliance-blocking-javascripts-not-working-3163235-4.patch1.13 KBsvenryen

Comments

John_B created an issue. See original summary.

john_b’s picture

john_b commented

In hook_js_alter() the line
_eu_cookie_compliance_convert_relative_uri($script);
has the opposite affect, and by converting relative paths to absolute ones prevents local JS tags from being unset.

john_b’s picture

john_b commented
Title: blocking extermal javascripts not working » blocking javascripts not working
Issue summary: View changes
svenryen’s picture

svenryen commented
Status: Active » Needs review
StatusFileSize
new eu_cookie_compliance-blocking-javascripts-not-working-3163235-4.patch1.13 KB

Hi @John_B! Nice catch. This was actually fixed in 8.x-1.x, but got never carried over to 7.x-1.x.

The attached patch should fix the problem.

john_b’s picture

john_b commented

Sorry, the patch does not fix the problem.

As per my comment #2, _eu_cookie_compliance_convert_relative_uri($script); is still running and converting relative paths to absolute ones, so relative paths do not get unset.

In addition, absolute paths are still getting converted to relative paths.

Suggestion:
1. remove _eu_cookie_compliance_convert_relative_uri($script); -- I am not sure what is there for?

2. to prevent the absolute links losing the protocol part before the :, this works, though not elegant:

      // Remove 'category:' if present.
      $parts = explode(':', $script);
      // Deal with absolute links with category.
      if (strpos($parts[2], '//') === 0) {
        array_shift($parts);
        $script = implode(':', $parts);
      }
      // Deal with absolute links without category.
      else if (strpos($parts[1], '//') === 0) {
        $script = $script;
      }
      // Deal with relative links.
      else {
        $script = end($parts);
      }
svenryen’s picture

svenryen commented

John B, what's the input that you use for the disabled scripts?

I tried key1:https://ajax.googleapis.com/ajax/libs/angularjs/1.4.4/angular.min.js, and I was successfully able to disable this script in 7.x.

With regards to [1], if we remove that line, we might have regressions.

john_b’s picture

john_b commented

What you say is correct. However, your patch does not work where there is no category.

In other words, your patch works for:
key1:https://ajax.googleapis.com/ajax/libs/angularjs/1.4.4/angular.min.js
but does not work for:
https://ajax.googleapis.com/ajax/libs/angularjs/1.4.4/angular.min.js.

With regards to [1], we still need a fix. For example I am loading
sites/all/modules/custom/my_custom_module/js/intercom.js
If in the config form I add
sites/all/modules/custom/my_custom_module/js/intercom.js
or
key1:sites/all/modules/custom/my_custom_module/js/intercom.js
the value of $script, after that function runs, is set to
https://example.com/sites/all/modules/custom/my_custom_module/js/intercom.js
(where example.com is my $base_url). The script, because it has a relative path is not blocked.

svenryen’s picture

svenryen commented

Thanks. Will look into cases when there's no category later today.

svenryen’s picture

svenryen commented
Status: Needs review » Needs work
svenryen’s picture

svenryen commented
StatusFileSize
new eu_cookie_compliance-blocking-javascripts-not-working-3163235-10.patch1.53 KB

Thanks for your input.

My IDE barfs at this line:
$script = $script;

If you agree, can we commit this instead?

      // Deal with absolute links with category.
      if (strpos(!empty($parts[2]) && $parts[2], '//') === 0) {
        array_shift($parts);
        $script = implode(':', $parts);
      }
      // Deal with relative links.
      else if (!empty($parts[1]) && strpos($parts[1], '//') !== 0) {
        $script = end($parts);
      }

It should give the same result.

I also found another related bug, so here's an updated patch.
Would be great if you can review, John_B.

svenryen’s picture

svenryen commented
Status: Needs work » Needs review
neslee canil pinto’s picture

neslee canil pinto
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

1. First i did checked this functionality working for 8.x branch with the latest dev version.
2. As @John_B had pointed out in #7 which is what how it should act. +1 on that scenario.

@svenryen yes, i agree on your latest patch and the new catch too. I think this is ready to commit into 7.x.

john_b’s picture

john_b commented

Sorry to have been absent. This project was on the backburner, & I was busy on other things. Since it is marked RTBC, it only remains to say thanks @svenryen and @Neslee Canil Pinto for work on this.

neslee canil pinto’s picture

neslee canil pinto
he/him
Primary language English
commented

@John_B did you had a look at the latest patch submitted by @svenryen. I did check it with your scenario at it did work for me

neslee canil pinto’s picture

neslee canil pinto
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed to dev branch.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.