(Note: I only consider this "critical" from the UI perspective -- the module is in fact working)

I've tried on both Safari 2.0.4 and FF 1.5 (I know, I should upgrade to 2.0), and the JS stuff on the Node queue tab seems to have a very serious flaw. I'll attach a couple of screenshots to illustrate what I'm seeing. Basically, if a node isn't in any queues, and I click on "Add to queue", the little spinning clock thing works as expected. However, when it's done, the page is otherwise unchanged. :( There's no indication that the node now belongs to that queue. So, the "nodequeue-js-woes-initial.png" screenshot I'm attaching here is exactly what you see before you click on that link, and after the spinning clock thingy goes away. However, if you refresh the page (click on the "Node queue" tab again), you now see the "In queue" column updated, and the "Operation" link is now "Remove from queue" (I'll attach a screenshot of that in the next followup).

This will be tremendously confusing for the folks that'll use the site I'm building based on nodequeue. Smartqueues are ideal for what I need, and the JS/AJAX stuff is nice to make it more responsive. However, if there's no feedback that it worked to add the node to a give queue, people are going to keep clicking the link (which will add duplicates into the queue), etc.

I'm not a JS/AJAX wizard, so I'm not sure exactly how to approach fixing this. But, it seems like you need to update the "In queue" and "Operation" cells in the table when that AJAX returns success.

Comments

dww’s picture

StatusFileSize
new94.3 KB

And here's the screenshot once you refresh the page... This is what it should look like after you click the link, but before a page refresh.

Side-note for testing/reproducing: these are subqueues generated via a taxonomy smartqueue, in case that happens to matter...

dww’s picture

Just to be clear: the same thing happens (or rather, nothing happens) with the "Remove from queue" link. The page still appears as if the node is in the queue, and you've still got a "Remove from queue" link which you can keep clicking (which only appears to make the clock thingy spin).

merlinofchaos’s picture

Does the same thing happen when using hook_link, i.e, on node links for teasers? (Or are you not using those?)

merlinofchaos’s picture

I don't have FF 1.5 anywhere, but I popped over to my Mac and tried this with Safari 2.04 and it seems to work reliably for me. I'm not sure why it's failing for you.

Do you have devel.module and has it been updated recently? (and I should make sure I'm setting the proper header so devel.module will drop its output for ajax)

merlinofchaos’s picture

Status: Active » Postponed (maintainer needs more info)

Nope, it's not devel; but keep in mind that older devel.module can cause this to happen. It's easy to see with firebug.

At this point I"m at a loss what might be causing the problem if your devel.module is up to date.

dww’s picture

Assigned: Unassigned » dww
Category: bug » support
Priority: Critical » Normal
Status: Postponed (maintainer needs more info) » Active

- updated devel.module to the end of DRUPAL-5: same problem
- disabled devel.module completly: same problem
- updated all of core to the end of DRUPAL-5: same problem
- reinstalled nodequeue module: same problem
- reset all menus and cleared all caches: same problem
- tried from another mac with FF 2.0.0.7: same problem
- tried it with a regular top-level nodequeue instead of a taxonomy-based subqueue: same problem
...

tried installing nodequeue on a test site on my laptop: everything works. ;)

*sigh*

So, something is bogus with the site running on my crappy shared hosting provider. :( No idea what it is, but it doesn't *appear* to be a bug in nodequeue...

I guess I'll try creating a new site completely from scratch on the shared hosting provider and see what happens. I'll update here once I know more, but probably there's nothing that needs to change in the code. Downgrading to a support request. Sorry for the false alarm...

-Derek

merlinofchaos’s picture

Before you go to that length, install firebug in your Firefox setup and see what, exactly, is being passed back to the browser during the AJAX call. That's a lot quicker than setting up a whole new site. My best guess is that something is appending data which causes the JSON parse to fail which causes nothing to apparently happen.

dww’s picture

Thanks for the pointer, good advice. I don't use firebug often, so I'm not super familiar with it (clearly I should learn). The "Console" tab when I clicked an "Add to queue" link shows this (all in red):

POST http://test5.dwwright.net/?q=admin/content/nodequeue/8/add/11/114&destination=node%2F114%2Fnodequeue&tab 404 (1536ms)

When I try to click on that link manually, it works, and I don't get any 404.

Any ideas?

merlinofchaos’s picture

Check your server logs; see what's actually being hit?

dww’s picture

Category: support » bug
Priority: Normal » Critical
Status: Active » Needs review
StatusFileSize
new629 bytes

error.log shows the badness:

[Thu Oct 11 15:50:46 2007] [error] [client 71.139.205.22] malformed header from script. Bad header=text/javascript: php.cgi, referer: http://test5.dwwright.net/?q=node/114/nodequeue

And lo, the nodequeue_js_output() JS handler is trying to set a bogus PHP header. ;) See attached patch. That solves all my problems.

I don't know if you're still maintaining the 5.x-1.* version of this module, but it looks like there's no Content-type header being set at all in nodequeue_page() in the codepath for the JS output. Set this to "to be ported" if you want a patch for that, or you could just add the same line I included in my patch here right before the drupal_to_js() on line 170 of revision 1.39.2.28 from the end of the DRUPAL-5 branch.

dww’s picture

StatusFileSize
new604 bytes

Bah, since it was easy, here's an equivalent patch for the end of the DRUPAL-5 branch.

dww’s picture

StatusFileSize
new1.25 KB

Upon further inspection, there are a few other spots you're setting a bogus PHP header. Use this patch for DRUPAL-5--2, instead. #11 is still ok for DRUPAL-5, since there was only one place you were generating JS output.

merlinofchaos’s picture

Status: Needs review » Fixed

Committed! Silly me, totally missed that content-type thing.

Anonymous’s picture

Status: Fixed » Closed (fixed)