Opened 12 years ago

Closed 11 years ago

#5803 closed defect (fixed)

dojo.getComputedStyle does not honor dojo.doc

Reported by: guest Owned by: sjmiles
Priority: high Milestone: 1.2
Component: HTML Version: 1.0
Keywords: getComputedStyle dojo.doc dojo.withGlobal Cc: tdedischew@…
Blocked By: Blocking:

Description (last modified by sjmiles)

dojo.getComputedStyle doesn't recognize changes to dojo.doc / dojo.withGlobal because it caches the defaultView on load:

dojo/_base/html.js:262 	var gcs, dv = document.defaultView;

If resolving dojo.doc.defaultView was/is an expensive operation I'm not sure what a good fix would be.

Adding a doc argument to getComputedStyle (like dojo.byId) could work, but this might be confusing since the standard W3C getComputedStyle uses a second arg already (pseudo element).

My temporary workaround is to just back out the cached dv reference:

if (!dojo.isIE) 
  dojo.getComputedStyle = eval(String(
    dojo.getComputedStyle.toSource ? 
      dojo.getComputedStyle.toSource() : 
      dojo.getComputedStyle.toString()
    ).replace(/ dv./g, ' dojo.doc.defaultView.'));

Note: I discovered this issue on FF 1.5 when trying to get the computed style.position value -- using the wrong document's defaultView did not pick up CSS rules applied using class= and always returned position as "static"

Attachments (1)

test_getComputedStyle.html (1.3 KB) - added by guest 12 years ago.
Test Case demonstration issue for FireFox? / Mozilla

Download all attachments as: .zip

Change History (7)

Changed 12 years ago by guest

Attachment: test_getComputedStyle.html added

Test Case demonstration issue for FireFox? / Mozilla

comment:1 Changed 12 years ago by guest

os/browser test results:

Win/Mac? FF 1.5 -- alerts "static" (incorrect) then "absolute" (correct w/fix):

Mac Safari 1.3.2 -- alerts "absolute" -- no need for fix

Win IE6 -- alerts "absolute" no need for fix (modified test to use document.frames[0].document instead of frame.contentDocument)

comment:2 Changed 12 years ago by guest

... this ticket should have been reported under my email -- gmail at tdedischew. I think this is a bug in Trac with using the Preview button (always resets email to guest) -- submitted a ticket for this too #5804

comment:3 Changed 12 years ago by dante

Cc: tdedischew@… added

comment:4 Changed 12 years ago by dylan

Milestone: 1.2

comment:5 Changed 12 years ago by sjmiles

Status: newassigned
If resolving dojo.doc.defaultView was/is an expensive 
operation I'm not sure what a good fix would be.

I'm having trouble assessing how expensive it is.

I assumed it was not particularly expensive, and was about to simply replace "dv" with "dojo.doc.defaultView", but I decided to run some tests because getComputedStyle is often at the heart of time-critical loops.

Profiling a style-insensive layout operation in one of my apps showed that the non-cached version takes 20% longer. However, the profiler reports that the dojo.getComputedStyle function is only using fraction of that time and cannot be blamed for the difference. This is a contradiction which I have to blame on bad profile data (IMO it's especially difficult to get good profiling info when manipulating DOM).

An alternative solution would be to have a function in html that could reset the cached "dv" value, and call that function from the context-setting API. It's not a particularly complicated solution, but certainly more involved than the dead-simple replacement. What gives me pause is that someone modifying dojo.doc directly, instead of using the context-setting API, could have confusing bugs as a result.

I think probably the more complicated solution is best, but I'm going to wait to see if there are any comments before I make it so.

comment:6 Changed 11 years ago by sjmiles

Description: modified (diff)
Resolution: fixed
Status: assignedclosed

This was fixed in r13951 by using node.ownerDocument.defaultView locally instead of any cached view.

It's possible this has a negative performance impact, but it seems worth it.

Note: See TracTickets for help on using tickets.