Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#11622 closed defect (fixed)

Reduce usage of dojo.marginBox in Dijit

Reported by: Shane O'Sullivan Owned by: Shane O'Sullivan
Priority: high Milestone: 1.6
Component: Dijit Version: 1.5
Keywords: Cc: bill
Blocked By: Blocking:

Description

dojo.marginBox can be a very expensive operation, particularly in IE6 and IE7. Often, when used in Dijit, the only properties that are needed are width and height, however the left and top values are also calculated.

They require accessing offsetLeft and offsetTop parameters of DOM nodes, which can cause severe performance issues in IE6 and 7.

This ticket introduces a new function, dojo._getMarginSize, which just returns the width and height, and changes Dijit code to use it where appropriate

Attachments (1)

marginBox.patch (10.1 KB) - added by Shane O'Sullivan 9 years ago.
Adds the dojo._getMarginSize function, and changes Dijit to use it

Download all attachments as: .zip

Change History (11)

Changed 9 years ago by Shane O'Sullivan

Attachment: marginBox.patch added

Adds the dojo._getMarginSize function, and changes Dijit to use it

comment:1 Changed 9 years ago by Shane O'Sullivan

Owner: set to Shane O'Sullivan
Status: newassigned

Added initial patch file, replacing dojo.marginBox usage with dojo._getMarginSize where appropriate.

If it is not ok to use a private dojo method in Dijit, a public method can be added, dojo.marginSize, which would necessitate adding a setter also.

comment:2 Changed 9 years ago by James Burke

For the _base/html.js change, it looks fine to me. It can be private for now, given that it is a new function, and there is only a need for a getter. Dijit's _Templated calls dojo._toDom() right now, so there is precedence for Dijit calling an underscore method.

comment:3 Changed 9 years ago by Shane O'Sullivan

(In [22765]) Refs #11622 Adds a new lightweight method, dojo._getMarginSize(), which returns the width and height of the margin box. This runs much more quickly on older browsers than dojo.marginBox, as it does not calculate the left and top values. Also changed the places in Dijit which use dojo.marginBox where they just require width and height to use this new function !strict

comment:4 Changed 9 years ago by Shane O'Sullivan

Resolution: fixed
Status: assignedclosed

Added dojo._getMarginSize and applied it throughout Dijit. Closing as fixed.

comment:5 Changed 8 years ago by bill

(In [25253]) dojo._getBorderBox() was removed in html.js refactor. Use dojo.position() instead.

Also, replace unnecessary use of private dojo._getMarginSize() with dojo.position().

Refs #9641, #11622, #13107 !strict.

comment:6 Changed 8 years ago by bill

(In [25254]) Replace unnecessary use of private dojo._getMarginSize() with dojo.position(). Refs #9641, #11622, #13107 !strict.

comment:7 Changed 8 years ago by bill

(In [25256]) comment usage of _getMarginSize(), refs #11622 !strict

comment:8 Changed 8 years ago by bill

(In [25257]) Simplify setting initial size of source view <textarea>. I think the old logic was wrong, but it didn't matter because _resize() was called right afterwards.

Also, since dojo.position(ed.domNode) returns the border-box size, we should not be looking at ed.domNode's margin size. Refs #9745, #11622 !strict.

comment:9 Changed 7 years ago by DJ Mountney

Looking at the last change [25257], you ended up with a double + on line 302

comment:10 Changed 7 years ago by bill

In [28577]:

fix typo, thanks twk3, refs #11622 !strict

Note: See TracTickets for help on using tickets.