How can this JavaScript snippet be improved?

Question

Hi,

This code (below) comes from an Open-source project (SemanticScuttle) I slightly modified the original code, attempting to convert a bookmarklet, into "web usable" Javascript.

Current Status: Google Chrome = Works perfectly!

Fiefox = Functional, but opens in a full sized tab, instead of the pre-defined size.

IE = DOA = Not functioning, please help...

    <script type="text/javascript">
var selection = '';
if (window.getSelection) {
    selection = 'window.getSelection()';
} else if (document.getSelection) {
     selection = 'document.getSelection()';
 } else if (document.selection) {
     selection = 'document.selection.createRange().text';
 }

 document.write('<a href="javascript:x=document;a=encodeURIComponent(x.location.href);t=encodeURIComponent(x.title);d=encodeURIComponent('+selection+');open(\'http://example.com/share/bookmarks.php/Energy?action=add&amp;amp;popup=1&amp;amp;address=\'+a+\'&amp;amp;title=\'+t+\'&amp;amp;description=\'+d,\'Site Name\',\'modal=1,status=0,scrollbars=1,toolbar=0,resizable=1,width=885,height=765,left=\'+(screen.width-885)/2+\',top=\'+(screen.height-725)/2);void 0;"><img src="images/bar/book22_5.jpg" alt="Example Code" title="Example Code" /></a>');

</script>

Since it works in Chrome and functions in Firefox, I would really Love to be able to use it with iE also. Thank you very much.

Answer 1

var selection;
if (window.getSelection) {
    selection = window.getSelection();
} else if (document.getSelection) {
     selection = document.getSelection();
 } else if (document.selection) {
     selection = document.selection.createRange().text;
 } else {
     selection = null;
 }

 if (selection !== null) {
    // Do your thing i.e. take out the stuff within the document.write 
    // and have it execute from here.
 }

Answer 2

The relevant portion of your code is (reformatted & abbreviated) as follows:

open( urlToOpen, 'Site Name', options );

It should read instead:

open( urlToOpen, '_blank', options );

The problem is with how you're using the window.open() function. IE thinks you're trying to open the url in a target called "Site Name". If you change it to "_blank" it will open in a new window.

Check the section about the sName parameter @ MSDN: http://msdn.microsoft.com/en-us/library/ms536651(VS.85).aspx

Answer 3

You could write a function to get the current selection as follows and then call it when you need it:

  function getSelectedText() {
     // get the selection function
     var selection = window.getSelection || document.getSelection 
                        || document.selection;

     if(selection)   {
        var range = selection.createRange;
        if(range) {
           selection = range();
        }
        // do something with selection
        return selection.text || selection();
     }
  }

The above works in all browsers (IE, FF, Chrome)

I havent tested this part but i think if you write your code one line at a time and then later join it, it would be easier to debug, here's an example (Warning! needs testing):

  var href = [
     'x=document;',
     'a=encodeURIComponent(x.location.href);',
     't=encodeURIComponent(x.title);',
     'd=encodeURIComponent(getSelectedText());', // call doSelection here.
     'url="http://example.com/share/bookmarks.php/Energy";',
     'opts="modal=1,status=0,scrollbars=1,toolbar=0,resizable=1,width=885,height=765,left=" + ((screen.width-885)/2) + ",top=" + ((screen.height-725)/2);',
     'open(url + "?action=add&popup=1&address=" +a + "&title="+ t + "&description=" +d ,',
        '"Site Name", opts);',
     'void 0;'
  ].join("");

document.write('<a href="' + href + '"><img src="images/bar/book22_5.jpg" alt="Example Code" title="Example Code" /></a>');