Opened 9 years ago

Closed 8 years ago

Last modified 7 years ago

#11465 closed enhancement (wontfix)

Wrap HTML5 classList API

Reported by: dylan Owned by: Eugene Lazutkin
Priority: high Milestone: 1.7
Component: Core Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

HTML5 has a new classList API that's currently supported in Firefox. We should test wrapping it and see if it improves performance.

See http://davidwalsh.name/classlist for examples and notes on the API.

Attachments (1)

html.js.patch (2.8 KB) - added by James Thomas 9 years ago.
Patch for html.js to add classlist support

Download all attachments as: .zip

Change History (13)

comment:1 Changed 9 years ago by Eugene Lazutkin

Milestone: tbdfuture
Owner: changed from anonymous to Eugene Lazutkin
Status: newassigned

Changed 9 years ago by James Thomas

Attachment: html.js.patch added

Patch for html.js to add classlist support

comment:2 Changed 9 years ago by James Thomas

I had a look at the performance impact of changing to support Element.classList at http://jsperf.com/native-dojo-class-manipulation-v-html5/2. This naive test showed a favourable improvement. WebKit? has recently implemented this feature, http://trac.webkit.org/changeset/68440, and Firefox supports this since 3.6.

I've patched html.js to support classList where possible, failing back to the previous implementation otherwise. I tested the patch against the class manipulation tests in tests/_base/html.html in FF (with support) and Chrome (without).

comment:3 Changed 9 years ago by Eugene Lazutkin

This is something we want to do, yet there are two problems to consider:

  1. The patch adds quite a few byte to the base.
  2. It tests dynamically if classList is available on every call --- it should be avoided.

It may be a good candidate for a core module, which replaces the base implementation, if possible. This way we retain the functionality, and when performance is critical we can load the fast implementation.

comment:4 in reply to:  3 Changed 9 years ago by James Thomas

Replying to elazutkin:

This is something we want to do, yet there are two problems to consider:

  1. The patch adds quite a few byte to the base.
  2. It tests dynamically if classList is available on every call --- it should be avoided.

It may be a good candidate for a core module, which replaces the base implementation, if possible. This way we retain the functionality, and when performance is critical we can load the fast implementation.

I agree on both points, it could be leaner and wonder whether we should be delegating to has.js for this in the future rather than testing ourselves?

I'm not sure it really fails into a core module though. I would imagine that toolkit users would prefer Dojo to load the most appropriate implementations for my platform, rather than relying on me to explicitly require it. Do we have any other core modules that follow this pattern?

comment:5 Changed 8 years ago by Eugene Lazutkin

Milestone: future1.7
Resolution: fixed
Status: assignedclosed

Done in [25704] of #9641.

comment:6 Changed 8 years ago by bill

(In [25717]) fix breakage from [25704], refs #9641, #11465

comment:7 Changed 8 years ago by bill

Note: original ticket was #9747.

comment:8 Changed 8 years ago by Eugene Lazutkin

Resolution: fixed
Status: closedreopened

It was reverted in [25745]. We decided to postpone it for now until vendors will ship better/faster implementations.

comment:9 Changed 8 years ago by Eugene Lazutkin

Resolution: wontfix
Status: reopenedclosed

comment:10 Changed 7 years ago by darth

Is this worth considering again? All major browsers in their latest incarnations now support this. Performance also seems to favor native implementation. Updated the above jsperf test with dojo 1.8 as well as using dojo and classList combo http://jsperf.com/native-dojo-class-manipulation-v-html5/7

Last edited 7 years ago by darth (previous) (diff)

comment:11 Changed 7 years ago by mm

Please If you have consideration about buggy (slow) browsers, can you contribute to http://jsperf.com/native-dojo-class-manipulation-v-html5/8 and show what browsers are known to be slow (i'm aware of some gecko bugs, but IMHO already fixed).

I have added sample has() detection and domClass.contains1() method.

Speed up is significant, and usage of domClass is quite heavy in dijit/dojox, so every optimization (usage of faster native) can help.

Worst case: make it as extension/optional even excluding native, from buggy browser versions (yes browser sniffing of needed). All still better than keeping slow version.

Please: Is this worth considering again?

comment:12 Changed 7 years ago by bill

I'm against doubling the size of dojo/dom-class.js just to make very fast browsers even faster.

Note: See TracTickets for help on using tickets.