Opened 16 years ago

Closed 16 years ago

#2254 closed Bug (fixed)

Malfunction in FCKSelection object

Reported by: Evgeny Satanovsky Owned by: Martin Kou
Priority: Normal Milestone: FCKeditor 2.6.2
Component: General Version: FCKeditor 2.5
Keywords: Confirmed Review+ Cc:

Description

FCKSelection have 2 methods called: HasAncestorNode and MoveToAncestorNode. Both of theme have input parameter called nodeTagName of type string and as written in comments of this function the passed string should be written in upper case.

But in the code when check is done actual tag name never converted to upper case.

if ( oContainer.tagName == nodeTagName ) return true ;

So if your edited HTML has tags written in lower case (like custom tags) this functions won't find the ancestor tag.

Hope you'll fix this small thing in next revision.

tank's

Attachments (3)

2254.patch (1.8 KB) - added by Wojciech Olchawa 16 years ago.
2254_2.patch (1.8 KB) - added by Martin Kou 16 years ago.
2254_3.patch (1.8 KB) - added by Martin Kou 16 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 16 years ago by Wojciech Olchawa

Keywords: Confirmed added; FCKSelecte upper case tagName HasAncestorNode MoveToAncestorNode removed

comment:2 Changed 16 years ago by Frederico Caldeira Knabben

Milestone: FCKeditor 2.6.1
Owner: set to Wojciech Olchawa

Changed 16 years ago by Wojciech Olchawa

Attachment: 2254.patch added

comment:3 Changed 16 years ago by Wojciech Olchawa

Keywords: Review? added

comment:4 Changed 16 years ago by Martin Kou

Keywords: Review- added; Review? removed
Owner: changed from Wojciech Olchawa to Martin Kou
Status: newassigned

Hmm... the patch has a number of problems:

  1. toLocalUpperCase() is not defined, which gives out JavaScript errors.
  2. Even converting the toLocalUpperCase() to toUpperCase(), the patch is still wrong because it still can't check against DOM nodes with lowercase tag names.

We actually have a shorthand string function defined in FCKeditor's code for this kind of case insensitive string matching - String::IEquals(). You can find it in $FCKeditor/editor/_source/fckjscoreextensions.js, it's a very useful function. :)

Changed 16 years ago by Martin Kou

Attachment: 2254_2.patch added

comment:5 Changed 16 years ago by Martin Kou

Keywords: Review? added; Review- removed

comment:6 Changed 16 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review+ added; Review? removed
Summary: Malfunction in FCK.Selecte objectMalfunction in FCKSelection object

BTW, I think that sometimes Safari does return node names with a different case than the rest of the browsers. Am I right?

Do you think that this can fix any other problem?

comment:7 Changed 16 years ago by Martin Kou

Hmm... no? Safari gives out upper case nodeNames for HTML nodes, just the same as any other browser at the moment.

comment:8 Changed 16 years ago by Martin Kou

Resolution: fixed
Status: assignedclosed

Fixed with [2046].

Click here for more info about our SVN system.

comment:9 Changed 16 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review+ removed
Resolution: fixed
Status: closedreopened

I've reverted [2046] with [2063]. The context menu was broken in IE.

Changed 16 years ago by Martin Kou

Attachment: 2254_3.patch added

comment:10 Changed 16 years ago by Martin Kou

Keywords: Review? added; Review- removed

Ok, tagName is not always defined for all DOM nodes, particularly for the #document node.

Changing to checking for nodeName instead.

comment:11 Changed 16 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:12 Changed 16 years ago by Martin Kou

Resolution: fixed
Status: reopenedclosed

Fixed with [2072].

Click here for more info about our SVN system.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy