Thursday, August 27, 2009

Fixing markitUp! 1.1.5 - bug in IE8 when closing preview iframe

[UPDATE - 1/12/2010]: MarkItUp! version 1.1.6 inclues a fix for this issue (thanks Jay!)

I've been working with the wonderful markitUp! editor by Jay Salvat. Specifically, I'm using markitUp! as a markdown editor for TicketDesk and a few other apps I'm working on. I'll probably post more about using markitUp! as a markdown editor later, but for now I wanted to address a specific bug that markitUp! exhibits in IE8.

By default, markitUp! uses an iframe element for a preview window.

In IE 8, closing the preview iframe will cause IE 8 to try to close the entire hosting window or tab. IE 8 will prompt the user before it does this, but if you click yes when prompted it will indeed kill the window/tab... which is not good.

After some digging, I've located the problem...

Here is the relevant code with the part that is called when closing the preview window in bold:

// open preview window
// open preview window
function preview() {
 if (!previewWindow || previewWindow.closed) {
     if (options.previewInWindow) {
  previewWindow = window.open('', 'preview', options.previewInWindow);
     } else {
  iFrame = $('');
  if (options.previewPosition == 'after') {
      iFrame.insertAfter(footer);
  } else {
      iFrame.insertBefore(header);
  }
  previewWindow = iFrame[iFrame.length - 1].contentWindow || frame[iFrame.length - 1];
     }
 } else if (altKey === true) {
       if (iFrame) {
   iFrame.remove();
      }
      previewWindow.close();
      previewWindow = iFrame = false;
 }
 if (!options.previewAutoRefresh) {
     refreshPreview();
 }
}
What is supposed to happen is that the code removes the iframe element, then calls the close method on previewWindow variable. That variable would normally have a reference to the content window within the iframe (you can see where that is set in blue in the code excerpt above). So calling close on the variable would normally just try to close that sub-window... or maybe it would do nothing at all because the iframe containing the sub-window would have already been removed. The behavior is internal to the browser and I suspect that specific mechanics are probably a little different from one browser to another. But either way, this works fine on all the browsers I've tested with except IE 8.

With IE8, this code appears to invoke the the close() call on the containing window instead (which is your page's main window in most cases). If you make the close call before the iframe is removed, IE8 will behave like the other browsers do, but when the close call happens after the iframe is removed IE 8 starts asking you if you want to close the whole browser window. for some reason, the contents of the previewWindow variable change after you remove the iframe.

Not a problem! my solution was to simply alter the code to only call the close method when there isn't an iframe being used. That way, close is called for cases where you are using the pop-up window, but doesn't get called when you are using an iframe.

// open preview window
 function preview() {
  if (!previewWindow || previewWindow.closed) {
      if (options.previewInWindow) {
   previewWindow = window.open('', 'preview', options.previewInWindow);
      } else {
   iFrame = $('');
   if (options.previewPosition == 'after') {
       iFrame.insertAfter(footer);
   } else {
       iFrame.insertBefore(header);
   }
   previewWindow = iFrame[iFrame.length - 1].contentWindow || frame[iFrame.length - 1];
      }
  } else if (altKey === true) {
          if (iFrame) {
     iFrame.remove();
        }        else {
     //SMR - else block added here to prevent this call when preview is in iframe
     //      IE8 incorrectly tries to close the hosting window if you call it when using iframe
     previewWindow.close();
        }
          previewWindow = iFrame = false;
  }
  if (!options.previewAutoRefresh) {
      refreshPreview();
  }
}

 This seems to fix the problem in IE 8, while not breaking anything in the other browsers I'm testing with (chrome, firefox, etc). I could have just moved the close call so it happened before the iFrame gets removed (which also seems to work), but I'm a little concerned that closing the window before removing the iframe might have unexpected results in browsers I'm not testing with... but it would probably be fine either way.

If you want, you can download my modified versions of the markitup! source. I have included a standard and minified one both.

Please note that this version also contains my own killPreview() function. By default markitUp! has only one toolbar button for the preview. If you click it, the preview opens and if you ALT + Click it the preview closes. But in my own implementations I prefer to have a separate toolbar button for closing the preview... users don't magically "know" about the ALT+Click trick and I get tired of people reporting "I can't close the preview window" as a bug in the apps that use markitUp!.


No comments:

Post a Comment