Ticket #5803 (assigned defect)

Opened 3 months ago

Last modified 3 months ago

dojo.getComputedStyle does not honor dojo.doc

Reported by: guest Owned by: sjmiles
Priority: normal Milestone: 1.2
Component: HTML Version: 1.0
Severity: normal Keywords: getComputedStyle dojo.doc dojo.withGlobal
Cc: tdedischew@…

Description

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

test_getComputedStyle.html (1.3 kB) - added by guest 3 months ago.
Test Case demonstration issue for FireFox? / Mozilla

Change History

Changed 3 months ago by guest

Test Case demonstration issue for FireFox? / Mozilla

Changed 3 months 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)

Changed 3 months 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

Changed 3 months ago by dante

  • cc tdedischew@… added

Changed 3 months ago by dylan

  • milestone set to 1.2

Changed 3 months ago by sjmiles

  • status changed from new to assigned
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.

Note: See TracTickets for help on using tickets.