Opened 14 years ago

Closed 14 years ago

#5022 closed Task (wontfix)

Suggestion: Move functions "is()" and "getName()" to node.js

Reported by: Scott McNaught Owned by:
Priority: Normal Milestone:
Component: General Version:
Keywords: Discussion Cc:

Description

Hello,

During my tinkering with ckeditor, I think it would be very convenient to have the methods "is()" and "getName()" moved from element.js to node.js.

This makes sense because nodeName is a part of the browsers TextNode object as well. It helps because if you are doing a test to see if an element is say a <br /> the code changes from:

if(element && element.type == CKEDITOR.NODE_ELEMENT && element.is('br'))
or:
if(element && element.is && element.is('br'))

to just:
if(element && element.is('br'))

I have seen these sort of checks a lot of places in the ckeditor source. Let me know your thoughts.

Change History (4)

comment:1 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Discussion added

We wanted to have a clean and useful DOM representation in our API. We're not based on the W3C DOM for that, but we try to make them similar whenever possible. But, we're also avoiding senseless things.

While your idea makes sense to make those checks a bit simpler, looking at each of the API classes separately could make it senseless. For example, if you have a text node with the text value "br", one could thing that textnode.is('br') should return "true". After all its a text (string) being compared with a string.

Then, getName() for text or comment nodes is again senseless, and it's a duplication of something that can be achieved through the "type" property.

Of topic... Regarding your code, I know you may find "element.is" to check for element nodes, but this is wrong. The "type" attribute should always be used for node type detection. I've already had a talk with the other guys about that.

comment:2 Changed 14 years ago by Frederico Caldeira Knabben

Type: BugTask

comment:3 Changed 14 years ago by Scott McNaught

Understand your point with the text node .is() being misunderstood to be the text content.. But... CKEDITOR.dom.text.is is undefined.

Maybe the method .is() should be renamed to .isA(), isName(), isNode() or isNodeName()?

Anyway - its not a big deal, just a suggestion as I was working with it :)

comment:4 Changed 14 years ago by Frederico Caldeira Knabben

Resolution: wontfix
Status: newclosed

There are no strong benefits on making changes to the API at this point. We'll keep this in mind if we find out it's a critical need.

Thanks for the suggestion anyway ;)

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