In http://drupal.org/node/983372#comment-3810044 (module Fieldset helper) was reported a bug with jQuery caused by the adding of .after(' ') in http://drupal.org/node/541568#comment-2237264 ; I meet the same problem without using Fieldset helper for which this problem was reported.

Under certain circumstances, .after(' ') in line 82 of collapse.js throws an exception "Syntax error, unrecognized expression: " (+ a space). Then everything using JS (wysiwyg CKeditor and Relations fieldset) isn't well displayed on Chrome, Firefox or Safari (see attached screencopy).

I have (systematically) this error on the term edit page, but curiously only one vocabulary is affected, the one which has the more fields (18 fields, in which 3 CKeditors).

Unfortunately I didn't achieve to reproduce this bug from scratch.

If I remove the .after(' ') in collapse.js my CKEditors are back but the relations fieldset is still broken.

Did anyone met this ? Thanks for helping.

See also : http://drupal.org/node/983372#comment-6029570
Original report : http://drupal.org/node/541568#comment-6029598

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nod_’s picture

Version: 7.14 » 8.x-dev

Looks like .after shouldn't be used to add a random string, has to be a dom element of some sort.

Since the selectors are too clever it's a pain to see where the space should actually be added and why. Is it possible to point out the before/after HTML and show where the space is supposed to be? there must be another way to add it that doesn't break everything.

In the meantime, can you try replacing .after(' ') by .after(document.createTextNode(' ')) and see if that works better?

pverrier’s picture

Version: 8.x-dev » 7.14
FileSize
225.63 KB
212.69 KB

Sorry, apparently I did not select the good core version, it's on 7.14.
-----

Here are 2 screenshots from Chrome debugger.

I can see that this code in collapse.js

      // Turn the legend into a clickable link, but retain span.fieldset-legend
      // for CSS positioning.
      var $legend = $('> legend .fieldset-legend', this);

      $('<span class="fieldset-legend-prefix element-invisible"></span>')
        .append($fieldset.hasClass('collapsed') ? Drupal.t('Show') : Drupal.t('Hide'))
        .prependTo($legend)
        .after(' ');

indirectly calls
set.push.apply( set, jQuery(arguments[0]).toArray() );
with arguments[0] = " "

So it looks for a selector " ", which throws the exception.
-----

I tried with .after(document.createTextNode(' ')) : no exception thrown, the CKEditors are well displayed but the Relations section is still not working.
-----

Also tried with .append(document.createTextNode(' ')), since apparently a space is to be appended (?) I'm not enough skilled in JQuery to understand the difference between append and after...
Same result.
-----

I noticed that the Relations fieldset is displayed between my 11th (image) and 12th (description) field, I don't know why it isn't at the end of all the fields... Then I had the idea to flip these two fields, then miracle, Relations is working... Flipping again : not working again... I've already noticed that the weights seem to be linked with this problem, but it's very strange...
-----

Thanks for helping !

nod_’s picture

Version: 7.14 » 8.x-dev

patch for D7 in #1607822: illeagal jQuery creator in misc/collapse.js can you please test pverrier?

Also the bugfixes go to the 8.x version first then are backported to 7.x.

nod_’s picture

Sounds like there is a bug in relations anyway, can you link to the module/js file so that I can take a look please?

greenrover33’s picture

Status: Active » Patch (to be ported)
FileSize
440 bytes

Here the patch for drupal-8.x-dev (2012-05-29)

nod_’s picture

Status: Patch (to be ported) » Needs review

Please read the status guidelines ;)

Status: Needs review » Needs work

The last submitted patch, illegal_jQuery_creator_in_misc_collapse_js.patch, failed testing.

pverrier’s picture

I confirm that by replacing .after(' ') by .after('<span> </span>') or .after(document.createTextNode(' ')) my CKEditors are well displayed.

But I also have another JS error caused by Google Analytics Counter "Uncaught TypeError: Property '$' of object [object Window] is not a function" (Chrome) or "$ is not a function" (FF) on

$(document).ready(function(){
setTimeout("gac_request_new_count()",7000);
});

This error was already here and I didn't made a link between the two problems, but the bug described in https://drupal.org/node/1607822 by greenrover33 seems of the same kind (?). Neither of the corrections solves this for me (but is it linked? no issue has been posted about this bug in the GAC module, apparently).

@Greenrover33, did this error disappear after your patch? Do you use GAC?

I progressed a little about the broken Relations fieldset and confirm it's linked with the field weights (but still don't know how): with the following hook, I changed the Relations weight, now it's well displayed (and displayed at the bottom of the page):

function MYMODULE_form_taxonomy_form_term_alter(&$form, &$form_state) {
  $form['relations']['#weight'] = 25;
}

I don't know why, but on certain weights, the fieldset is broken and inactive. It occurs when the weight of 'relations' is the same as CKEditor fields, or Image widgets, but it's not directly linked to these types, and only in indexes between 7 and 10. I don't understand, but my hook curiously fixes this bug...

I wonder why the weight of 'relations' is only of 10. As now we can add fields in taxonomies, the relations fieldset appears at the 10th position, and before the following weighted fields (if it occurs). Perhaps this weight should be increased?

nod_’s picture

Try with

jQuery(document).ready(function () {
  setTimeout(gac_request_new_count, 7000);
});

Sure sounds like the relations things is not related to this issue. If for some weights it's working it's unlikely that core code is the one messing it up.

greenrover33’s picture

Againe the same patch but formated by git in hope that the automated test wont fail this time.

This issue here: https://drupal.org/node/1607822 i found cause of problems with ctools modal frames ajax reloads.
But user:nod_ told me to post it for drupal 8 insted of 7. Then it will be backported. I only like to get this patch in drupal 7 ;-)

But i`am a good gue and downloded the google_analytics_counter-7.x-1.1.tar.gz
And reviewed it.
http://drupal.org/node/1608470

greenrover33’s picture

Priority: Normal » Major
Status: Needs work » Needs review
greenrover33’s picture

Priority: Major » Normal

Status: Needs review » Needs work
nod_’s picture

can you go with document.createTextNode(' ')? I'd like to avoid changing the html and adding a useless span element here just to make jQuery happy.

Also please post the google analytics patch in the google analytics queue, this doesn't need to be in a core issue.

Thanks for your help :)

greenrover33’s picture

No i cant, there you would mixe plain js and jquery. This is nerver a good idea.
Then span will annoy nobody.

An .after("&nbsp;") would also be not HTML conform and fail.

nod_’s picture

Well putting a var space = document.createTextNode(' '); up top and adding it with .after(space) would avoid mixing them directly. There is nothing wrong with mixing the two when jQuery is insufficient for the task at hand.

But as you'd like, in the end that file will end up being rewritten. This chaining feast needs to be cleaned up.

(edit) and you never know what contrib is up to. Might be that someone has some crazy css rules on span elements at this place that will mess things up for him. In doubt, don't change the markup.

(edit') Please change it.

ksenzee’s picture

I'm not sure I follow #16, but it is definitely better to create a text node and add that with jQuery after() than it is to add a spurious span element.

pverrier’s picture

@greenrover33 many thanks for your patch #10 for Google Analytics Counter, it works !

Also thanks for creating issue in GAC (http://drupal.org/node/1608470).

romaingar’s picture

I've got the same problem on firefox/ie when i use a fieldset with no title.
the $legend varible could not be set and the next selector fail with jquery.

var $legend = $('> legend .fieldset-legend', this);

      $('<span class="fieldset-legend-prefix element-invisible"></span>')

change to

var $legend = $('> legend .fieldset-legend', this);

if(!$legend.length)return;

      $('<span class="fieldset-legend-prefix element-invisible"></span>')

maybe check why there is no title to your fieldset..

nod_’s picture

Status: Needs work » Needs review
FileSize
538 bytes
nod_’s picture

Status: Needs review » Reviewed & tested by the community

should not have been .after(' ') to begin with, can and should be backported.

nod_’s picture

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed e4783d5 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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

mcpuddin’s picture

What's the best way to get this backported? To add a new patch here or to create a new issue?

nod_’s picture

Version: 8.x-dev » 7.x-dev
Status: Closed (fixed) » Patch (to be ported)

like that :)

  • alexpott committed e4783d5 on 8.3.x
    Issue #1606110 by greenrover33, nod_: Fixed Exception thrown in collapse...

  • alexpott committed e4783d5 on 8.3.x
    Issue #1606110 by greenrover33, nod_: Fixed Exception thrown in collapse...

  • alexpott committed e4783d5 on 8.4.x
    Issue #1606110 by greenrover33, nod_: Fixed Exception thrown in collapse...

  • alexpott committed e4783d5 on 8.4.x
    Issue #1606110 by greenrover33, nod_: Fixed Exception thrown in collapse...
Jill L’s picture

Issue summary: View changes
Status: Patch (to be ported) » Needs review
FileSize
553 bytes

4 years later, here's a backport :)