Opened 8 years ago

Closed 7 years ago

#862 closed defect (fixed)

Adding target to a link on an image doesn't work on creation

Reported by: charles@… Owned by: gogo
Priority: normal Milestone: Version 1.0
Component: Xinha Core Version: trunk
Severity: normal Keywords:
Cc:

Description

When inserting a new link, the "target" property isn't set when doing it with IE (haven't tried with Firefox yet). Strange thing is, when modifying the link, you can alter the target correctly.

I have tried with the live example and the same behavior occurs

I checked the code but cannot pinpoint the exact line where things go bad

Change History (6)

comment:1 Changed 8 years ago by koto

  • Milestone set to Version 1.0

I confirm this behaviour.

Testcase:
just use Xinha example on IE browser (I use IE7 RC1), select some text in the first paragraph and try to create the link. The target and title element will not get applied.

Debugging the code shows that the _createlink function exits at this point (before assigning the link properties):

      if ( ! ( a && a.tagName.toLowerCase() == 'a' ) )
      {
        return false;
      }

In my case, variable a was a 'P'DOM element (paragraph), so there is something wrong in creating the anchor object in IE. Something is probably wrong in this try..catch block:

        try
        {
          editor._doc.execCommand("createlink", false, param.f_href);
          a = editor.getParentElement();
          var sel = editor._getSelection();
          var range = editor._createRange(sel);
          if ( !HTMLArea.is_ie )
          {
            a = range.startContainer;
            if ( ! ( /^a$/i.test(a.tagName) ) )
            {
              a = a.nextSibling;
              if ( a === null )
              {
                a = range.startContainer.parentNode;
              }
            }
          }
        } catch(ex) {}

Variable a after this block is the same as before (p DOM element).

comment:2 Changed 8 years ago by ray

Adding target works in FF, but only when link created does not span over more than one node. That may be a related issue.
I think the best solution would be, _createLink employ the same method to create link implemented in the Linker plugin

      // Insert a link, we let the browser do this, we figure it knows best
      var tmp = HTMLArea.uniq('http://www.example.com/Link');
      linker.editor._doc.execCommand('createlink', false, tmp);

      // Fix them up
      var anchors = linker.editor._doc.getElementsByTagName('a');
      for(var i = 0; i < anchors.length; i++)
      {
        var a = anchors[i];
        if(a.href == tmp)
        {
          // Found one.
          for(var j in atr)
          {
            a.setAttribute(j, atr[j]);
          }
        }
      }
    }
    linker.editor.selectNodeContents(a);

This ensures that all attributes are applied to all created link nodes.

comment:3 Changed 8 years ago by ray

So this should fix it

Index: htmlarea.js
===================================================================
--- htmlarea.js	(revision 593)
+++ htmlarea.js	(working copy)
@@ -3967,20 +3967,20 @@
       {
         try
         {
-          editor._doc.execCommand("createlink", false, param.f_href);
-          a = editor.getParentElement();
-          var sel = editor._getSelection();
-          var range = editor._createRange(sel);
-          if ( !HTMLArea.is_ie )
+          var tmp = HTMLArea.uniq('http://www.example.com/Link');
+          editor._doc.execCommand('createlink', false, tmp);
+
+          // Fix them up
+          var anchors = editor._doc.getElementsByTagName('a');
+          for(var i = 0; i < anchors.length; i++)
           {
-            a = range.startContainer;
-            if ( ! ( /^a$/i.test(a.tagName) ) )
+            var a = anchors[i];
+            if(a.href == tmp)
             {
-              a = a.nextSibling;
-              if ( a === null )
-              {
-                a = range.startContainer.parentNode;
-              }
+              // Found one.
+              a.href =  param.f_href;
+              if (param.f_target) a.target =  param.f_target;
+              if (param.f_title)  a.title =  param.f_title;
             }
           }
         } catch(ex) {}

Can you confirm?

comment:4 Changed 8 years ago by charles@…

Your patch fixes the target issue but, if there is another link in the edited document, the focus is sent to that one instead of the one which was being edited after the "Ok" button is pressed

(Tested with SVN HEAD Full-exeample)

comment:5 Changed 8 years ago by ray

Yes, you are right. Fixed this and gonna commit now, though there is still a problem when editing a link that spans over more than one node (impossible at the moment). I'll try to get that one later, as it will need some DOM trickery.

comment:6 Changed 7 years ago by ray

  • Resolution set to fixed
  • Status changed from new to closed

committed in rev 595

Note: See TracTickets for help on using tickets.