I noticed on d.o that the JS which runs when you change the project on an issue (to update the version and component fields with the new choices based on the project you select) had stopped working. I went to the URL manually and noticed the page timer from devel module getting printed out. I just went into the devel settings page and disabled the pager timer, and now the JS is working great again.

I guess this is really a bug for devel.module, but I wanted to submit it first to the infra queue, so everyone knows why the page timer is now off. I guess it's possible that the menu callback in project_issue that's used by the JS code is written in such a way that it's vulnerable to this bug. It's in issue.inc, project_issue_update_project(). The function just creates some output (rendered form elements, to be precise) and then does this:

  print drupal_to_js($return);
  exit();

Is that not the recommended way for JS callbacks to work anymore? Moshe, any words of wisdom on this?

Please advise.

Thanks,
-Derek

CommentFileSizeAuthor
#6 project_issue_update_project_js_header.patch.txt879 bytesdww

Comments

ilo’s picture

ilo commented

It's not only devel module, but all modules attaching text to the output from the server. So even if fixed in this module, other modules may break the javascript behavior as well.

Anyway it's the devel feature to attach that page/node/database information to any request submitted by the browser to help in development, so I beleive there is nothing to be fixed here. If you don't want any text before the javascript than exiting is a good solution. Other solution could be to put the exit right into drupal_to_js function, but I think it may break many other drupal features.. In any case, if the devel module should not attach any text to the request if it's a javascript return, how will that javascript generation be debugged?

dww’s picture

dww
we/he/they
commented

@ilo: It seems you didn't read the code snippet I posted. Lemme try again:

  print drupal_to_js($return);
  exit();

I don't see how I could possibly try to get my output (and only my output) printed any more clearly than that. The problem is that devel.module is registering an on_exit handler to print that timer from inside the call to exit() itself. Your comment about "exiting is a good solution" misses the point.

In any case, if the devel module should not attach any text to the request if it's a javascript return, how will that javascript generation be debugged?

It'd be one thing if we were talking dvm() or dpr(), etc (devel functions for printing debug messages). But, this is a generic feature of "show me how long it took to render this page". The debug output itself renders the page useless in the case of JS callbacks.

I see a few options:

A) JS callbacks have to do something to disable all the on_exit handlers.
B) devel.module provides a setting for paths to exclude on_exit handler output on (sort of like the block visibility setting).
C) devel.module is somehow smart enough to know about JS callback functions (e.g. a hook whereby modules can register their JS callbacks with devel?)
D) No one ever turns on the devel module on a site that uses any JS callbacks.
E) Some other bright solution that Moshe comes up with. ;)

(A) and (D) both seem completely unfeasable. (C) is probably way too complicated to actually work. That leaves (B) and (E)...

Crell’s picture

Crell commented

I'm not Moshe :-), but I've been pondering for Drupal 7 having a menu callback declare what type it is: page, json, ahah, rss, etc. Then "sideband" content (blocks, exit handlers, etc.) can read that value and decide if they should do anything differently. In this instance that would allow devel's shutdown handler to do something like:

$item = menu_get_active_item();
if ($item->return_type == 'page') {
  // Print various debug output
}

As a bonus, we could also then get rid of the silly exit() in the non-page menu callbacks because blocks and page theming and other sideband stuff could "know" that they're not supposed to do anything. No help right now, but something that I've been bouncing around that would offer more flexibility to sideband content. I'll probably be babbling more about it at DrupalCon. :-)

pwolanin’s picture

pwolanin commented

Derek,

I think I saw this issue before and perhaps Moshe will confirm. I think what you need to do is follow the pattern in the D6 function drupal_json():

http://api.drupal.org/api/function/drupal_json/6

By setting a JS header before calling drupal_to_js() the Devel module knows to NOT append output to your content.

merlinofchaos’s picture

merlinofchaos commented

drupal_set_header('Content-type: text/javascript');

Then devel won't touch it.

dww’s picture

dww
we/he/they
commented
Project: Drupal.org infrastructure » Project issue tracking
Version: » 5.x-2.x-dev
Component: Drupal.org module » Miscellaneous
Assigned: Unassigned » dww
Status: Active » Needs review
StatusFileSize
new project_issue_update_project_js_header.patch.txt879 bytes

Thanks folks. Sounds like a plan.

hunmonk’s picture

hunmonk commented
Status: Needs review » Active

tested as working. committed to HEAD w/ an added code comment for clarity. this still needs to be installed on d.o, so setting back to active

dww’s picture

dww
we/he/they
commented
Status: Active » Fixed

Installed on d.o, page timer re-enabled, and issue JS still working. ;)

moshe weitzman’s picture

moshe weitzman commented

the notion that menu callbacks declare their content type might be useful. that conversation is probably best located at http://drupal.org/node/145551 (Enable dynamic page loading and rendering into different formats ...)

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)