It took me a bit to nail this down, but when I use AdSense Injector with Drigg, it prints out the injected code for the ad immediately prior to executing it. This could have been anything - an Ajax problem, a Digg module quirk, Filtered HTML filtering improperly - but I spotted this similar problem in a seemingly unrelated Drupal issue with the Garland theme:

http://drupal.org/node/102252#comment-269363

And so I tried using the 4plus theme, and the problem disappeared. It's theme-based, and it's the default Drigg theme. You seem to have done a lot of work recently getting various media types embeddable in Drigg - posting a code snippet to help the default theme get along with AdSense Injector would be tremendous if you could.

The problem occurs in both full-page views and Teasers. And only in Drigg modules - in a Story, for example, it displays correctly even under the default theme.

CommentFileSizeAuthor
StoryInjection.gif5.12 KBSatori42
DriggInjection.gif10.91 KBSatori42
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mercmobily’s picture

Hi,

I am afraid I can't help you here.
I have never installed the Injector, I know nothing about CSS, and I wouldn't have a clue on even starting tackling this problem...

I would suggest opening an issue with the Adsense injector people. But... I am not sure if they would be able to help either.

Finally, _all_ of the efforts at the moment are into porting Drigg to D6. The Adsense Injector doesn't exist for D6...

Please have a look at the generated HTML and see if you can figure out the problem. If not... I will close it as won't fix.

Sorry...

Merc.

Satori42’s picture

I opened an issue with the Injector people, before realizing that it was a Drigg default theme problem. Given that neither of us are familiar with CSS, my best bet would probably be to take it to Drigg's forums. Should Injector release a Drupal 6 port, would you like me to notify you again provided the problem remains?
And is there any possibility that Drigg will support this in its Drupal 5 version after the port to 6 is complete?

Be well,

- Satori

mercmobily’s picture

Hi,

I feel very guilty closing this issue when the problem still persists. It doesn't feel right.
Can you try and have a look on Drigg's forums, and report back here as soon as you've found something out?

Thanks for your help,

Bye,

Merc.

Satori42’s picture

I've poked around on the Drigg forums, not finding anything on this. Where would you like me to open a thread about it, in "Installation", "Customization", or "Theming"?

mercmobily’s picture

Hi,

I'd say customisation.

Merc.

Satori42’s picture

Done. For reference, the thread is here:
http://www.drigg-code.org/node/537

Thanks again. This is certainly a very involved module to author, but it's tremendous!

mercmobily’s picture

Hi,

OK!

Merc.

Satori42’s picture

I've examined the problem and found out what it was, and a fix that works. Now we just need an elegant, "official" solution that won't interfere with anything else - and that will keep problems like this from cropping up elsewhere in Drigg's default theme, because it's currently throughout the theme. Please see the forum thread for more information.

Be well,

- Satori

mercmobily’s picture

Hi,

I have the feeling we should move the forum thread here, and not vice-versa.
The forum thread is for support, not really bug fixing.
Can you please paste your post here, and put a link from the forum thread to here?

Bye!

Merc.

Satori42’s picture

Title: Default Drigg theme throws weird code with AdSense Injector » Problem solved - needs an official solution.

I've located the problem and found a stopgap solution for myself. To make this simpler for the cavalry when they get here, I'm going to attempt to explain the bug as it was found in the Garland case - for which a solution was found - because it explains what's happened here.

The Garland CSS was essentially unique in that it had script in its header. When AdSense attempted to insert a banner there via javascript, the theme displayed the javascript itself before executing it.

The team looking into the bug realized that this was due to code in the Garland theme's style.css file which read:

284 #header-region * {
285 display: inline;
286 line-height: 1.5em;
287 margin-top: 0;
288 margin-bottom: 0;
289 }

Not only was the "display: inline" unnecessary (because it's the default setting), and never mind that Garland's theme was the only theme to use that setting, the "*" in line 284 applied the inline setting to absolutely everything there... including unexpected AdSense script.

What "display: inline;" does, evidently, is cause whatever it is (in this case everything, AdSense script and all) to be displayed in a way that tries to fit it all on one line as best it can - to make best use of space. Because the Garland theme - and Drigg's default theme - approach things this way, AdSense Injector comes along independently trying to inject script for an AdSense ad into the Drigg module. The "display: inline" code throws the fact that AdSense code is normally invisible right out the window, does a stiff little formal salute and proceeds to format the code to fit it on one line as best it can - formatting evidently being prioritized ahead of code execution - and within a Drigg module the text wraps within the block anyway once it's out of space. Then, it proceeds to execute itself - displaying the AdSense ad after all... after merrily babbling out the script itself onto the screen.

The crew fixing the Garland theme bug solved the problem by commenting out the "display:inline;" and a few more formatting lines immediately following it as well, before deciding that this was dangerously kludgey and could produce unanticipated side-effects down the road. The problem was then re-solved by simply putting, "display:none;" immediately after the whole thing.

The solver then suggested that the "*" applying itself to anything and everything that appeared in that area was dangerously and needlessly clumsy, and should probably be replaced with only those things specifically that ought to be formated as inline.

I've compared the CSS between the 4plus theme and Drigg's default theme. Drigg's default uses lots of "display: inline;"s throughout, and has problems with AdSense Injector. 4plus does neither.

I've resolved the problem by removing these three lines from the nodes.css file of Drigg's default theme:

.node.ntype-drigg .story * { display: inline; }
.node.ntype-drigg .story small a { color: #666; }
.node.ntype-drigg .story p { margin: 0; }

(They're just above the "Embedded content" section), saving the file and replacing the themes/drigg_theme/css/nodes.css with that copy. I had to make sure my site wasn't using a cache, and I closed all browser tabs with my site in them, clearing my browser's cache before going back into my site.

It worked.

Be well,

- Satori

Satori42’s picture

mercmobily’s picture

Hi,

Quick question: is this safe to do in the default theme?
Or shall I not, and leave some instructions to people using the injector instead?

Merc.

Satori42’s picture

With neither of us knowing CSS, that's an open question. You may want someone with more CSS familiarity to look into it, but I'm inclined to think it should be fine - and even an improvement.

From what I've read, "display: inline;" is the default setting and isn't necessary in the first place. All adding it does is force everything to be displayed on one line as best it can... even things that ordinarily aren't displayed. For example, the "Read >>" and "Read more >>" links end up getting split onto the next line when there isn't enough room for them. I consider this unnecessarily unaesthetic. The use of "display: inline;" is apparently not standard CSS practice, and occurs liberally throughout the default Drigg theme. That in itself is probably dangerous - unanticipated modules attempting to work with Drigg and add other content within the Drigg module will very likely have problems with this in the future, when using the default theme. The Drigg community will probably want to take care of this problem when it can, to prevent people from tripping over it later on.

The Drigg CSS in my post #10, above, was incorrect. It should have read:

.node.ntype-drigg .story * { display: inline; }
.node.ntype-drigg .story small a { color: #666; }
.node.ntype-drigg .story p { margin: 0; }

...and removing the first of those lines from themes/drigg_theme/css/nodes.css alone takes care of it. It /should/ be safe, since the formatting seems unaffected other than solving the problem - because inline is the default value, and all the line does is explicitly require even script that's normally invisible to be displayed. And that is not desired in the first place.

Potential drawbacks? I suppose if someone went to post a code snippet as a Scoop - which isn't unlikely, one of my uses for Drigg will be to allow people to share snippets of code - that code snippet might be executed rather than displayed.

The optimal solution would be to change the * in the first line I've cited to refer specifically to things that Drigg is looking for... Drigg modules? Other stuff?... rather than applying the inline property to everything en masse. But I don't know what Drigg looks for. You may. Any thoughts?

mercmobily’s picture

Hi,

I will need to check with our graphic designer guru... I'll get back to you!

Merc.

mercmobily’s picture

Title: Problem solved - needs an official solution. » Default Drigg theme throws weird code with AdSense Injector
Status: Active » Fixed

Hi,

Well, this is clearly a web design issue, and I don't think we need to change the default theme. I think we are good.

Merc.

Status: Fixed » Closed (fixed)

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