Very slow / delayed fade in with overlay on Firefox

Removing the opacity will remove the loading image in FF and Opera. I'll try to find another solution for that.

I'm also affected by this. The overlay just got visibly slower for me (instead of faster).

It concerns the following code in Drupal.overlay.load()

  // When a new overlay is opened and loaded, we add a loaded class to
  // the dialog. The loaded class is not removed and added back again
  // while switching between pages with the overlay already open,
  // due to performance issues.

  //self.$dialog.removeClass('overlay-loaded');
  self.$iframe
    .css('opacity', 0.2)
    .load(function () {
      self.isLoading = false;

      // Only continue when overlay is still open and not closing.
      if (self.isOpen && !self.isClosing) {
        self.$iframe.css('opacity', 1);
        self.$dialog.addClass('overlay-loaded');
      }
      else {
        self.destroy();
      }
  });

The iframe is constructed together with the rest of the overlay code, the loading of the content in the iframe is what actually takes the longest. The spinner is a background image on a container for the iframe, so to show the spinner we need to lower the opacity of the iframe. The iframe can't be hidden with display:none, because our timer function checks the height of the iframe every 150ms, and with display:none the height would be 0 (resulting in flashing between page loads).

However, using visibility:hidden should do the same as lowering the opacity to 0.

Attached are 2 patches, both have the visibility instead of opacity change, but patch A also has //self.$dialog.removeClass('overlay-loaded'); uncommented. I'd especially like David Rothstein to try out both patches, since he had problems with the spinner before. In light of this issue, I'd like to know if it's the combination gif + opacity change, or just the gif that is causing performance issues for him.

I think 1a is working. The opacity was causing a little slow down for me as well, especially on large pages.

I'm assuming testbot is broken and not my patch? :)

I tested before and after the patch, and its much smoother.

#9 dereine, which one, a or b?

i tested the first one.(a)

Just readding patch 1a so testbot gives green light. 1a works for me and dereine. Anybody else?

I'd like to hear from David Rothstein, because he had performance issues with the loader gif before... If he still has issues, patch 1b would be a good temporary alternative.

I tried to apply #15 but it doesn't apply for me.

Hm. Interesting. I did see this yesterday before committing but assumed it was because I had 20,000 applications open.

#15 also doesn't apply for me. Marking needs work.

Re-rolled against HEAD.

status >.<

Well, I seem to have become the canary in the coal mine when it comes to the overlay :)

I tested both patches in #19 and they both perform equally well for me. (I'm not sure I ever had performance issues that were actually due to the loader gif..... mostly it seemed they were due to all the crazy JS selectors that you folks managed to remove.)

In any case, this patch definitely seems like an improvement. With the current HEAD, I am indeed seeing the slow fade-ins that others are reporting -- not as bad as it was in my earlier report that @catch refers to at the top of this issue, but still enough to be annoying in certain places.

Thanks David, 2a is the way to go then! Someone else wanting to review this and moving this closer to RTBC?

*Pokes randomly with stick, hoping to get a response* ;)

I didn't experience any sluggishness, but it's just weird to have it sit at 0.2 opacity untill the page is loaded, if for any reason it would take a while to load this would be extremely weird

so ++ as far as I'm concerned

Great. I confirm this fixes the problem here, too. I interpret seutje's and David Rothstein's comments as an RTBC from someone who knows front-end stuff better than I do. :)

Committed to HEAD!

I'm not sure I really know what I'm doing either, but RTBC is definitely how I interpret @seutje's comments ... and the code did look correct :)

However, I believe this makes the previous code comments obsolete? Here is a stab at fixing that.

Err, that got lost in the re-roll. Yes that indeed makes the old code comment obsolete!

visibility could be handled in CSS. Also updated comment.

Reroll

I tried to test this patch, and everything seems fine with it applied, but then I didn't notice any performance problems without it either (Ubuntu FF 3.5.6). Actually, I don't see any fading at all with or without this patch. What is supposed to be fading?

Also, a question which is probably not related to this issue, but this issue does mention the spinner graphic so I'll ask here anyway... The loading spinner graphic only appears every other time an overlay page is loaded. eg. Click the People tab and I see the spinner while the overlay loads. Click another top level link no spinner, another link spinner, another link no spinner. The spinner should either show every time the overlay loads or not at all.

#30, there is no fade animation, the visibility is toggled.

About the spinner: It's not showing every other time. The location of the spinner is 50% - 50%, so dead center of the overlay. If your overlay is really large, you won't see it.

That, or the other page just loads really fast so that you don't even get a chance to see the spinner.

This is a followup patch, not going to improve performance very much (although a little) any more. It's just cleaner.

About the spinner; the spinner is centered, so if you switch from a large page the spinner may fall outside of your viewport. You should open a new issue for this.

Re the spinner: #676008: Overlay's Loading 'spinner' graphic often not visible because it is displayed outside the viewport

Committed to CVS HEAD. Thanks for the follow-up.