Ticket #2254 (closed Bug: fixed)

Opened 3 months ago

Last modified 3 months ago

Malfunction in FCKSelection object

Reported by: jenka1980 Owned by: martinkou
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

2254.patch (1.8 kB) - added by w.olchawa 3 months ago.
2254_2.patch (1.8 kB) - added by martinkou 3 months ago.
2254_3.patch (1.8 kB) - added by martinkou 3 months ago.

Change History

Changed 3 months ago by w.olchawa

  • keywords Confirmed added; FCKSelecte upper case tagName HasAncestorNode MoveToAncestorNode removed

Changed 3 months ago by fredck

  • owner set to w.olchawa
  • milestone set to FCKeditor 2.6.1

Changed 3 months ago by w.olchawa

Changed 3 months ago by w.olchawa

  • keywords Review? added

Changed 3 months ago by martinkou

  • keywords Review- added; Review? removed
  • owner changed from w.olchawa to martinkou
  • status changed from new to assigned

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 3 months ago by martinkou

Changed 3 months ago by martinkou

  • keywords Review? added; Review- removed

Changed 3 months ago by alfonsoml

  • keywords Review+ added; Review? removed
  • summary changed from Malfunction in FCK.Selecte object to Malfunction 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?

Changed 3 months ago by martinkou

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

Changed 3 months ago by martinkou

  • status changed from assigned to closed
  • resolution set to fixed

Fixed with [2046].

Click here for more info about our SVN system.

Changed 3 months ago by fredck

  • keywords Review- added; Review+ removed
  • status changed from closed to reopened
  • resolution deleted

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

Changed 3 months ago by martinkou

Changed 3 months ago by martinkou

  • 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.

Changed 3 months ago by fredck

  • keywords Review+ added; Review? removed

Changed 3 months ago by martinkou

  • status changed from reopened to closed
  • resolution set to fixed

Fixed with [2072].

Click here for more info about our SVN system.

Note: See TracTickets for help on using tickets.