Doesn't work on page with other jquery

Yep i confirm this, actually i was wondering why sevenup was not working when testing with IE6. I do have a CODA slider on my frontpage, which results in the module not firing and my site dispalys plain ugly under IE6..

Any pointers on that?

Cheers,

Simon

Ya, I can confirm this too. I also have a jQuery slider on my homepage.
I get a js error that expects a } and points to a position in the code that makes no sense.
This error does not occur without Seven Up installed.

I've now removed the jQuery slider (and everything else) from my homepage inside the php (so the js never knows about it) yet the error persists.
After digging a bit into the outputted source I noticed this:
sevenup.test(options);(options);

now, I'm not a javascript ninja but this doesn't seem right...

here's the code surrounding it:

...
$(document).ready(function(){
   var options = {
      ......
   };
   sevenup.test(options);(options);
});
...

Problem solved!

The error isn't in the sevenup.test(options);(options); line after all (although i still think it makes no sense). The problem is a classic one of quotes canceling each other out.

In the message body there is an <h3> tag that contains a style attribute:
<h3 style="margin-top: 40px">
this style attribute has to go, so does any attempt to apply any attributes in the message body. (unfortunately that includes classes too)

here's the conflict (pulled from view source):

messageBody: "<h3 style="margin-top: 40px"> Why should I upgrade?</h3> <ul>  <li> <b>Websites load faster</b>, often double the speed of this ol...................."

as you can see the quotes around margin-top are problematic.

also, it's not so possible to switch to the single quotes in the php because both single and double quotes are already used in the string .. here is what i mean: here is the code that outputs the js into the html:

sevenup.module: ln203 (right at the end of the code)

  drupal_add_js('$(document).ready(function(){
    var options = {  
      enableClosing: '. variable_get('sevenUp_enableClosing', 'true') .',
      enableQuitBuggingMe: '. variable_get('sevenUp_enableQuitBuggingMe', 'true') .',
      overlayColor: "'. variable_get('sevenUp_overlayColor', '#000000') .'",  
      lightboxColor: "'. variable_get('sevenUp_lightboxColor', '#ffffff') .'",
      borderColor: "'. variable_get('sevenUp_borderColor', '#6699ff') .'",
      '. (variable_get('sevenUp_downloadLink', 1) ? '' : 'downloadLink: "http://www.getfirefox.com",' ) .'
      overrideLightbox: false,
      lightboxHTML: null,
      showToAllBrowsers: '. variable_get('sevenUp_showToAllBrowsers', 'false') .',
      messageHeading: "' . variable_get('sevenUp_messageHeader', t('Your web browser is outdated')) . '",
      messageEasilyUpgrade: "' . $easily_upgrade_text . '",
      messageBody: "' . $message_body . '",
      exploreOtherHeading: "' . variable_get('sevenUp_exploreOtherHeading', t('Explore other browsers')) . '",
      messageQuitBuggingMe: "' . variable_get('sevenUp_messageQuitBuggingMe', t('Remind me again after seven days')) . '"
    };
    '. $test_function . '(options);
  });', 'inline');

and the code that sets $message_body. it is from sevenup_init():

    $message_body = preg_replace( "/\s\s+/", ' ', variable_get('sevenUp_messageBody', t(
        "<h3 style='margin-top: 40px'>Why should I upgrade?</h3>
          <ul>
            <li><b>Websites load faster</b>, often double the speed of this older version</li>
            <li><b>Websites look better</b>, so you see sites they way they were intended</li>
            <li><b>Tabs</b> let you view multiple sites in one window</li>
            <li><b>Safer browsing</b> with phishing protection</li>
          </ul>")) );

so this block tries to set the $message_body from the settings variable 'sevenUp_messageBody' and if it doesn't find the variable, then it sets it to this chunk of code. this code is identical to the defaults in the settings screen.

so the workaround solution is to remove the style="margin-top: 40px" attribute from the <h3> tag both in the backoffice settings page for sevenup and in sevenup.module in the sevenup_init() function.

also, there are many style tags in the messageBodyBlack string building block. I assume it would be necessary to remove all of these to use that theme.

I would suggest NOT applying colors, etc to the text in the backend editor as they will come out in style tags.. go for CSS (you're going to have to do this without the class="" attribute ugh)

Hope this helps somebody :)

Ok, here is a rar file containing these modifications. I made them to both the default and black themes and factored the css out from inline to a sevenup.css file that's included in the module. A couple issues. the h3 tag that says "Why should I upgrade?" has a top margin of 40 in the regular theme and 0 in the black theme. It's not possible to set them individually since the sevenUpLightbox shares the same css id (sevenUpLightbox) in both themes. I'd fix this but i'm on the clock so i just compromised at 20px and they both look great.
also, the same element had a color of #ffffff applied for the black theme but that would make the text invisible on the white theme so i just took it out. it shows in black on the white theme and light gray on the black theme. Good enough imo.

also, the overlay does not show properly for the white theme on my machine (it doesn't show at all for the black theme, i think that's by design). It seems to be because the height is set to 100%, locally i fixed this with an absolute height (height: 2600px;) but i didn't want to submit that environment specific CSS in this patch. Just letting everyone know that that may be necessary.

If someone wants to modify this module so that the white and black themes' lightboxes come out with different IDs then this could be solved but it looks pretty good right now.

Also, these mods were made on the patched version that includes editable message text (Make text editable).

so that's it. Community, please review and approve this revision of Seven Up that includes editable message text (not my work, already reviewed) and fixed the js/jQuery bug (my work, needs review).

Changed status to fixed

actually probably better to change the status to needs review, although i'm pretty sure that it's working properly

Has someone committed this patch or does it still need to be applied on the 0.3 version of the module?