Ticket #6641 (closed Bug: fixed)

Opened 3 years ago

Last modified 3 years ago

Copy and pasting a newly created link breaks

Reported by: joolee Owned by: Saare
Priority: Normal Milestone: CKEditor 3.5.1
Component: General Version: 3.0
Keywords: Oracle Cc:

Description

To recreate the problem:

  1. enter new text.
  2. select the text and create a link.
  3. copy the link and paste it on another part of the editor.

Result: href gets stripped out.

Browser: FF 3.6.12 OS: Mac OS X 10.6.4

Attachments

6641.patch (389 bytes) - added by garry.yao 3 years ago.
6641_2.patch (2.5 KB) - added by garry.yao 3 years ago.
6641_3.patch (3.3 KB) - added by Saare 3 years ago.

Change History

comment:1 Changed 3 years ago by garry.yao

  • Status changed from new to confirmed
  • Version changed from 3.4.2 to 3.0
  • Milestone set to CKEditor 3.5.1

Link href was initially done as "javascript:void(0)/*323*/" for unknown reason, which cause it been converted into a text node after then.

Changed 3 years ago by garry.yao

comment:2 Changed 3 years ago by garry.yao

  • Status changed from confirmed to review
  • Owner set to garry.yao

comment:3 Changed 3 years ago by alfonsoml

  • Status changed from review to review_failed

I think that the idea about using such special href was to avoid problems with the browsers and this change might cause some unexpected problems.

I would review if it's still needed the

			var attributes = { href : 'javascript:void(0)/*' + CKEDITOR.tools.getNextNumber() + '*/' },
				removeAttributes = [],
				data = { href : attributes.href },

Because these patch seems to override that part, and in

					for ( i = 0 ; i < links.length ; i++ )
					{
						if ( links[i].href == attributes.href )
						{
							links[i].id = data.adv.advId;
							break;
						}
					}

we could change the "links[i].href == attributes.href" part to compare the _cke_saved_href instead.

comment:4 Changed 3 years ago by garry.yao

I've traced back to the root ticket of that though still find no context of it (sad there's no comment of it), do you mean the original hack was for the 'id' application?

The problem is that browser has no knowledge of "_cke_saved_href " so with a real "href" the clipboard link still not functioning.

comment:5 Changed 3 years ago by alfonsoml

I'm not sure about why it's done currently that way, I remember that it FCKeditor something similar was done in order to call the browsers "createLink" command and then get back all the created links.

And back to the patch:

As I said, if we're gonna set the real href, I don't see a reason to initiate the href property in those two objects with fake data, it can be simplified.

And just in case the browser modifies the href under certain conditions (IE6, some firefoxes, relative/absolute paths, ...) if we change the second check so it doesn't use href but instead a property that they won't touch, then we don't have a risk of a regression.

comment:6 Changed 3 years ago by gav

Just an FYI, the patch as currently described applied to 3.5.x trunk does seem to help Mac OSX 10.6.5 Firefox 3.6, but breaks Mac OS X 10.6.5 Safari 5.0.3. The resulting source after the paste is:

This is where <a href="http://www.google.com">a link</a> goes.<a href="undefined">a link</a>

The second link is just a paste of the first right after creating it, then going to View Source.

For a point of interest, the current release version has a similar problem in Safari. Using http://ckeditor.com/demo:

<p>
	This is where <a href="http://www.google.com">a link</a> goes.<a href="">a link</a></p>

The second link was just the immediate paste of the first one. This definitely seems like this is related to the existing bug, even though the resulting output is a touch different.

comment:7 Changed 3 years ago by wwalc

  • Keywords Oracle added

Changed 3 years ago by garry.yao

comment:8 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

I now understand that the original purpose of having a unique (faked) href is to make sure the adv:id application successful (to not considering any existed link of the same href), but this should be done instead inside the styles plugin.

comment:9 Changed 3 years ago by Saare

  • Status changed from review to review_failed
  • Copying and pasting a link results in ID duplication.
  • The fix must be ported to the image dialog (link tab) as well.

comment:10 Changed 3 years ago by Saare

Another TC:

  • Create a link with url 'test'.
  • Right click on the link -> link properties.
  • Change the URL to 'example' and click OK.
  • Copy the link and paste it somewhere in the editor.
  • Right click on the new link -> link properties.

Expected: URL field should show "example". Actual: URL field shows "test".

Changed 3 years ago by Saare

comment:11 Changed 3 years ago by Saare

  • Owner changed from garry.yao to Saare
  • Status changed from review_failed to review

Id duplication will be handled with #6915.

comment:12 Changed 3 years ago by garry.yao

  • Status changed from review to review_passed

comment:13 Changed 3 years ago by Saare

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

Fixed with [6290].

Note: See TracTickets for help on using tickets.
© 2003 – 2012 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy