/scripts/ad.js shows up with an error when using the IMCE browser. Basically, the IMCE browser doesn't have a valid ad ID or drupal user ID for ad.js to use when it posts back to the ad stats script.

The error shows up as:

Uncaught TypeError: Cannot read property 'UserID' of undefined in line 21 of ad.js

The line is:

postData['uid'] = Drupal.settings.ad.UserID;

Basically, when you're in the IMCE browser, there is no UserID in that DOM because no ads will ever show up there.

My solution was basically to check for the /imce path and to not post data when that happens but I don't understand why ad.js is even there in the first place? Is there a way in the module to have it only work on paths where there are actual content types being shown and not something like IMCE?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

abenamer created an issue. See original summary.

abenamer’s picture

Issue summary: View changes
abenamer’s picture

Here's a possible patch. I don't know why the ad module is calling itself in a context where IMCE is being used but this is a quick workaround for now. Basically, it doesn't call the post that sends advertisement data back to Drupal if Drupal.settings are undefined.

Unitoch’s picture

Status: Active » Needs review
FileSize
990 bytes

I can confirm that this issue exists, but I don't think the patch in #3 addresses the issue, or at least it doesn't for me. The approach in the patch looks like
if (!(Drupal.settings.ad.UserID) || !(Drupal.settings.ad.ServePath)) {

That looks to me like "If you can't get the UserID then proceed" which looks backwards to me. Here's a new patch that says "If you can get the ad, then proceed."

I'm still not really sure if this is the correct method to fix this, but it's at least a start for somebody else looking for a quick fix.

abenamer’s picture

Hi @unitoch,

That's the problem I was having as well with the code. There is no UserID or ServePath when you're using IMCE and either one seems to indicate the possibility that you're on IMCE. I thought it would be better to document either possibility in the code and not to leave one uncommented in the conditional. Either way, both our patches feel like we're fixing things at the front-end when this should somehow be resolved in the .module file. Shouldn't this module be aware of paths like /imce that won't have a UserID or ServePath on them?

Unitoch’s picture

Yeah, @abenamer, I agree with you. These seem like bandaids on the front end, and probably this should be dealt with earlier than the .js file.

darol100’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
456 bytes

I think we should be using the patch from #3. Adding an interdiff for the maintainer of this project can compare both patches.

  • Jeremy committed 42c8b04 on 7.x-3.x authored by abenamer
    Issue #2677498 by abenamer, Unitoch, darol100: Causes a javascript error...
Jeremy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, going with #3.

Status: Fixed » Closed (fixed)

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