Opened 9 years ago

Closed 9 years ago

#2113 closed Bug (fixed)

Safari: <span class="Apple-style-span"> created when inserting special characters

Reported by: martinkou Owned by: martinkou
Priority: Normal Milestone: Safari Compatibility
Component: General Version: FCKeditor 2.6
Keywords: Confirmed Safari Review+ Cc:

Description

Reproduction procedure:

  1. Open sample01.html in Safari.
  2. Apply "Red Title" style to the paragraph.
  3. Insert a few special characters via the special character dialog in the paragraph.
  4. The inserted characters aren't red and come with a strange <span class="Apple-style-span"> tag.

This seems to be a WebKit bug as the problem also appears when adding special characters via document.execCommand('inserthtml', false, $HTML) directly in Safari. But a short term workaround may be possible as well.

Attachments (4)

2113.patch (1.5 KB) - added by martinkou 9 years ago.
2113_2.patch (2.4 KB) - added by martinkou 9 years ago.
2113_3.patch (2.3 KB) - added by martinkou 9 years ago.
2113_4.patch (2.6 KB) - added by martinkou 9 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 9 years ago by martinkou

  • Summary changed from <span class="Apple-style-span"> created when inserting special characters to Safari: <span class="Apple-style-span"> created when inserting special characters

I've filed a bug report to WebKit at https://bugs.webkit.org/show_bug.cgi?id=18423.

Changed 9 years ago by martinkou

comment:2 Changed 9 years ago by martinkou

  • Keywords Review? added

comment:3 Changed 9 years ago by martinkou

  • Keywords Review? removed

Retracted the review request because a similar bug has been found in another dialog (fck_paste.html). Instead of adding a kludge in individual dialogs, it seems fixing the issue in FCK.InsertHtml() or in the document processor would make more sense now.

Changed 9 years ago by martinkou

comment:4 Changed 9 years ago by martinkou

  • Keywords Review? added

Updated the patch such that the fix is generalized to fix all FCK.InsertHtml() calls.

comment:5 Changed 9 years ago by martinkou

  • Owner set to martinkou
  • Status changed from new to assigned

comment:6 Changed 9 years ago by alfonsoml

  • Keywords Review- added; Review? removed

I think that getElementsByTagName returns a live collection, so it's automatically updated whenever the content of the document changes, so in order to properly remove some node, the for must be done backwards, or it could miss the next node:

 	for ( var i = spans.length -1; i >= 0 ; i-- )

Changed 9 years ago by martinkou

comment:7 Changed 9 years ago by martinkou

  • Keywords Review? added; Review- removed

Updated the patch according to Alfonso's suggestion.

comment:8 Changed 9 years ago by alfonsoml

  • Keywords Review- added; Review? removed

Sorry, but I've remembered right now that Safari does support getElementsByClassName so we could use that instead of getting all the spans, and iterate through them to find which ones does have that class.

Another nice feature that might be useful for this kind of tasks is the Selectors API (I think that IE8 also does support it but I'm not sure right now)

Sorry :(

comment:9 Changed 9 years ago by martinkou

The article you quoted was posted on December 21st, 2007... which is uncomfortably recent - Safari 3.0 was released before that. Are you sure the document.getElementsByClassName() method would run correctly for uses who didn't update their Safari browsers after 3.0? I don't really know as it seems there's no way I can find and install an older version of Safari 3.

I've changed the patch to use document.getElementsByClassName() if it is available and use the old DOM/JS method if not. This both gets the speed advantage of the getElementsByClassName() and ensures it would still work in case older Safari 3 doesn't support it.

Changed 9 years ago by martinkou

comment:10 Changed 9 years ago by martinkou

  • Keywords Review? added; Review- removed

comment:11 Changed 9 years ago by alfonsoml

  • Keywords Review+ added; Review? removed

Yes, I think that before 3.1 that method shouldn't be available and your patch does a nice job to take care of it.

I don't think that there will be too many people using the 3.0 versions, but it's better to be safe and avoid support requests due to this change.

comment:12 Changed 9 years ago by martinkou

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

Fixed with [1951].

Click here for more info about our SVN system.

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