/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?
Comment | File | Size | Author |
---|---|---|---|
#7 | interdiff-2677498-3-4.txt | 456 bytes | darol100 |
#4 | ad_imce.patch | 990 bytes | Unitoch |
#3 | fix-for-imce-2677498-3.patch | 1.01 KB | abenamer |
Comments
Comment #2
abenamer CreditAttribution: abenamer as a volunteer and commentedComment #3
abenamer CreditAttribution: abenamer as a volunteer and commentedHere'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.
Comment #4
Unitoch CreditAttribution: Unitoch as a volunteer commentedI 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.
Comment #5
abenamer CreditAttribution: abenamer as a volunteer and commentedHi @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?
Comment #6
Unitoch CreditAttribution: Unitoch as a volunteer commentedYeah, @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.
Comment #7
darol100 CreditAttribution: darol100 as a volunteer and commentedI think we should be using the patch from #3. Adding an interdiff for the maintainer of this project can compare both patches.
Comment #9
Jeremy CreditAttribution: Jeremy at Tag1 Consulting commentedThanks, going with #3.