Opened 5 years ago

Last modified 5 years ago

#12023 confirmed Task

Improve performance of element.find() and element.findOne()

Reported by: Piotrek Koszuliński Owned by:
Priority: Normal Milestone:
Component: General Version:
Keywords: Cc:

Description

These methods have to set a temporary id on the element because querySelectorAll has different behaviour that anyone could expect and it applies the selector to entire DOM tree, even below the root.

Additionally, querySelectorAll is not as fast as its simpler friends - getElementsByTagName and getElementsByClassName.

It may be worth to test such addition to find and findOne:

		if ( selector.match( /^\.[a-z_-]+$/i ) ) {
			var found = this.$.getElementsByClassName( selector.slice( 1 ) )[ 0 ];
			return found ? new CKEDITOR.dom.element( found ) : null;
		} else if ( selector.match( /^[a-z_-]+$/i ) ) {
			var found = this.$.getElementsByTagName( selector )[ 0 ];
			return found ? new CKEDITOR.dom.element( found ) : null;
		}

This needs a research, but I'm setting milestone, because we're planning to work on #10903.

Change History (1)

comment:1 Changed 5 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.5.0
Status: newconfirmed

I thought about two things:

  1. querySelectorAll returns a non-live nodes list, when getElementsBy* return a live list. It's unsafe to change type of find()'s return value. Therefore we can only make this improvement in findOne()
  2. We don't use find() and findOne() so often yet. We use them mostly in widgets and it may only become costly with growing number of widgets used in editors. Nested widgets will cause this, but not so fast to worry about it now. Therefore I remove the milestone.
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy