IMCE ajaxError handler leads to WSOD. [#1645728]

Reproducing this bug is not easy. I did it with a fresh install of Drupal Commons plus Editable fields, jQuery update, IMCE WYSIWYG bridge, and their dependent modules. Then create a new wiki page and try to preview it before saving. I reliably get a White Screen of Death (WSOD).

What happens is that my collection of modules conspires to generate an HTTP 404 error on an AJAX request. I do not know why IMCE feels that it has to respond to all AJAX errors, but that is what this line from imce.js does:

$(document).ready(imce.initiate).ajaxError(imce.ajaxError);

Stepping through the code, I see two things. First of all (though I noticed it second) the imce.initiate() function bails out at the line

  if (imce.conf.error != false) return;

and so it never calls imce.updateUI() and it never defines imce.msgBox. Secondly, imce.ajaxError() makes an innocuous-looking call to imce.setMessage(), which contains this code:

  var $box = $(imce.msgBox);
  // skip two lines ...
  $box.queue(function() {
    $box.css({opacity: 0, display: 'block'}).html(msg);
    $box.dequeue();
  });

As I step through the code, the WSOD occurs when I get to the $box.css() line.

I suggest three fixes. First, do not register a global ajaxError handler. Second, rewrite imce.setMessage() so that it tests for whether imce.msgBox is defined. Third, work out the initializing code so that it is more reliable.

If you want, I will write a patch for the second suggestion.

Comment	File	Size	Author
#5	imce-wsod-1645728.patch	1.43 KB	benjifisher

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

ufku CreditAttribution: ufku commented 20 June 2012 at 16:32

Status:

Active

» Postponed (maintainer needs more info)

Why is imce.js included in a regular page?

Comment #2

benjifisher

he/him or they/them

English

Boston area

CreditAttribution: benjifisher commented 20 June 2012 at 20:09

Status:

Postponed (maintainer needs more info)

» Active

I wrote,

Then create a new wiki page and try to preview it before saving.

In other words, I am looking at the page node/add/wiki. When I click the Preview button, the same page reloads.

Does that answer your question?

Comment #3

ufku CreditAttribution: ufku commented 21 June 2012 at 06:21

No.
imce.js should be included at /imce path only. There is something wrong with your IMCE integration.

Comment #4

benjifisher

he/him or they/them

English

Boston area

CreditAttribution: benjifisher commented 21 June 2012 at 11:13

Thanks for the clarification. As I said, this bug is hard to reproduce. I will look into it.

Comment #5

benjifisher

he/him or they/them

English

Boston area

CreditAttribution: benjifisher commented 21 June 2012 at 13:11

File	Size
imce-wsod-1645728.patch	1.43 KB

I think the IMCE Wysiwyg bridge module loads imce.js on node-edit pages (assuming that at least one field is configured to use a WYSIWYG editor). There is just one function in its .module file, which starts out

/**
 * Implementation of hook_wysiwyg_plugin().
 */
function imce_wysiwyg_plugin($editor, $version) {
  static $integrated = array();

  if (!module_invoke('imce', 'access')) {
    return;
  }
  // Load our invocation scripts.
  if (empty($integrated)) {
    $imcepath = drupal_get_path('module', 'imce');
    $path = drupal_get_path('module', 'imce_wysiwyg');
    drupal_add_js($imcepath . '/js/imce.js');
    drupal_add_js($imcepath . '/js/imce_set_app.js');
    drupal_add_js($path . '/js/imce_wysiwyg.js');
  }
  // ...

I have not looked into where hook_wysiwyg_plugin() is invoked, but this seems like the answer. (I grepped for "imce.js" and this is the only place I found it other than in the imce module folder.)

Come to think of it, that function should be named imce_wysiwyg_wysiwyg_plugin(), right? But that suggestion should go on a different issue queue.

I have attached a patch as I suggested at the start. Even though the right fix might be to change the imce_wysiwyg module, I think my patch is pretty innocuous. (It applies cleanly on the 6.x-2.x branch and with an offset of 1 line on the 7.x-1.x branch.)