Problem/Motivation

When a page contains a collapsible fieldset element, such that $('fieldset.collapsible') contains elements, and a fragment with special characters is in the url, such as http://mysite.com/#badfragment=broken, a javascript error will be thrown: Uncaught Error: Syntax error, unrecognized expression: .error, #badfragment=broken.

This issue happens in collapse.js near line 63:

var anchor = location.hash && location.hash != '#' ? ', ' + location.hash : '';
if ($fieldset.find('.error' + anchor).length) {
  $fieldset.removeClass('collapsed');
}

Notice that the anchor is being directly pulled from the URL and used in the selector. However, '.error' + anchor will not yield a valid selector, resulting in an error being thrown (breaking all JS on the page).

Proposed resolution

Not sure the best way to fix this. Ideally, the fragment for collapsed fieldsets would be namespaced in the URL so that it wasn't pulling ANYTHING that is a fragment. So perhaps have it look for #collapsed=NAME_OF_ELEMENT.
A simpler/quicker fix would be to check if the selector is valid, or do some regex replacement on special (invalid selector) characters.

Files: 
CommentFileSizeAuthor
#7 issue-2395065-d8.patch607 bytesPol
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,808 pass(es). View
#5 issue-2395065.patch594 bytesPol
PASSED: [[SimpleTest]]: [MySQL] 41,510 pass(es). View

Comments

rooby’s picture

A work around that might work for your case is this:

Instead of

if ($fieldset.find('.error' + anchor).length) {
  $fieldset.removeClass('collapsed');
}

Use this

      if (anchor.indexOf('?') == -1) {
        // Default behavior.
        if ($fieldset.find('.error' + anchor).length) {
          $fieldset.removeClass('collapsed');
        }
      }
      else {
        // Exclude the anchor if it contains a query string because otherwise
        // jQuery errrors out in IE.
        if ($fieldset.find('.error').length) {
          $fieldset.removeClass('collapsed');
        }
      }
samhassell’s picture

This conflict comes up when using AngularJS's $location service within D7.

Sample URL:

http://mysite.com/map#?station=8005&soil=3&groundCover=1
Pol’s picture

Same here too.

Pol’s picture

In misc/collapse.js, replace:

if ($fieldset.find('.error' + anchor).length) {

With:

      if ($fieldset.find('.error' + anchor.replace(/[=%;,\/]/g, "_")).length) {

If I get good feedback, I'll propose a patch for D7 core.

Pol’s picture

Status: Active » Needs review
FileSize
594 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,510 pass(es). View

Here's the patch.

tim.plunkett’s picture

Version: 7.x-dev » 8.0.x-dev
Status: Needs review » Needs work
Issue tags: +needs backport to D7

This code is unchanged in D8, meaning that it should probably be fixed there first.

Pol’s picture

Status: Needs work » Needs review
FileSize
607 bytes
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,808 pass(es). View

Here's the patch for D8.

longwave’s picture

Is [=%;,\/] the full set of invalid characters, or could there be others? If a replacement takes place, will it ever match anything useful, or match something else by mistake?

Maybe this is more readable/useful:

if (anchor.search(/[=%;,\/]/) == -1 && ...) {

or

if (anchor.search(/^[valid characters]*$/) != -1 && ...) {

markcarver’s picture

Component: forms system » javascript
Status: Needs review » Needs work
Issue tags: +JavaScript

Is [=%;,\/]the full set of invalid characters, or could there be others?

There could be others.

I would much rather the route of checking for expected/valid characters as is done in
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21Util...

That being said, I would imagine that we could abstract this and just add it to drupal.js as a utilitarian method:

Drupal.cleanId = function (input) {
  if (typeof input !== 'string') return input;
  // Replace certain characters with hyphens and lowercase.
  var output = input.replace('/[\s_\[]', '-').toLowerCase();
  // Strip all non-alphanumeric characters.
  output = output.replace(/[^A-Za-z0-9\-_]/, '');
  // Removing multiple consecutive hyphens.
  return output.replace(/\-+/, '-');
};

Then we could just do something like:

var anchor = Drupal.cleanId(location.hash && location.hash !== '#' ? ', ' + location.hash : '');

ron_s’s picture

Thank you for everyone's work on this issue. A more generic approach is definitely the way to go. We ran into this problem with AddThis. AddThis adds anchor tags on URLs when recommended content links are clicked. The rendered URL displays something like the following:

http://example.com/node/25#at_pco=smlwd-1.0&at_si=5622dc8ca8da4ad0&at_ac=per-2&at_pos=0&at_tot=1

The #5 patch appropriately handles the equal character ("="), but does not replace the ampersand ("&"). The result is a jQuery error:

Uncaught Error: Syntax error, unrecognized expression: .error_ #at_pco=smlwd-1.0&at_si=5622dc8ca8da4ad0&at_ac=per-2&at_pos=0&at_tot=1 (jquery.min.js:2)

If we add "&" to the possible characters in Patch #5 it works correctly. Therefore a patch that handles any possible special symbol would be the best method.

David_Rothstein’s picture

I just marked #2651230: collapse.js breaks when the hash contains jQuery BBQ-like query string as duplicate. There is a patch in that issue also.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.