Opened 10 years ago

Closed 9 years ago

#9842 closed defect (wontfix)

dojo.fx problem with object with style=”display: none;”

Reported by: sweb Owned by: dante
Priority: high Milestone: 1.6
Component: fx Version: 1.3.2
Keywords: dojo.fx style display Cc:
Blocked By: Blocking:

Description

hail i have a big problem with dojo and must use jQuery instead of dojo just for this bad bug: when i use dojo fx with inline css the dojo work good:

<html>
<head>
    <title>wipeIn test</title>
    <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/dojo/1.3.2/dojo/dojo.xd.js"></script>
    <script type="text/javascript">
        dojo.require("dojo.fx");

        dojo.addOnLoad(function() { 
				dojo.fx.wipeIn({node: 'target', duration: 500}).play();
        });
    </script>
</head>
    <body>
        <div id="target" style="display:none; background-color: #444; color: #fff; height: 400px; width:300px;">
            <br />Unnecessary animation :-)
        </div>
    </body>
</html>

bug: but when i use the css in style tag or in css file and import in my document the dojo faild to play animation:

<html>
<head>
    <title>wipeIn test</title>
    <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/dojo/1.3.2/dojo/dojo.xd.js"></script>
    <script type="text/javascript">
        dojo.require("dojo.fx");

        dojo.addOnLoad(function() { 
    			dojo.fx.wipeIn({node: 'target', duration: 500}).play();
        });
    </script>
    <style>
    	#target {
    		display:none; background-color: #444; color: #fff; height: 400px; width:300px;
    	}
    </style>
</head>
    <body>
        <div id="target">
            <br />Unnecessary animation :-)
        </div>
    </body>
</html>

Attachments (2)

bug-9482.html (1.4 KB) - added by Brian Arnold 9 years ago.
A simple test file
9482.patch (1.9 KB) - added by Brian Arnold 9 years ago.
A patch to resolve this bug

Download all attachments as: .zip

Change History (10)

comment:1 Changed 10 years ago by strife25

it looks like when the style is set to be inline, node.style returns nothing.

comment:2 Changed 10 years ago by strife25

correction, when the CSS styles are placed into the <style> tags instead of the actual node.

comment:3 Changed 10 years ago by dante

Milestone: tbd1.5
Owner: changed from Bryan Forbes to dante

comment:4 Changed 10 years ago by sweb

how much longer to fix this bug?

comment:5 Changed 9 years ago by Adam Peller

Milestone: 1.51.6

comment:6 Changed 9 years ago by Brian Arnold

I created a sample that demonstrates the bug above, and I think I have a decent fix for it, using getComputedStyle to get around the fact that the node's style doesn't contain display or visibility information. I do also have a CLA on file.

Changed 9 years ago by Brian Arnold

Attachment: bug-9482.html added

A simple test file

Changed 9 years ago by Brian Arnold

Attachment: 9482.patch added

A patch to resolve this bug

comment:7 Changed 9 years ago by Brian Arnold

I was talking to Bryan Forbes about this today, and noticed a few things:

  • I was dyslexic in numbering my file names. Whoops.
  • It doesn't cover everything. He linked me to https://github.com/bryanforbes/uber.js/blob/master/src/html.js#L162-197 as a form of visibility check that is more thorough.
  • While this patch does fix the issue presented in this ticket, it's definitely not performant. I didn't like the addition of an element to the body to check a default style, but felt like I should be thorough.

So, if this doesn't make it in, I'm not worried, but thought I'd contribute what I could. :)

comment:8 Changed 9 years ago by dante

Resolution: wontfix
Status: newclosed

Thank you for the patch, but I agree the added code and expense to a core function might not be worth it. "isvisible" is entirely too abstract a concept :) (parent display:none, etc) computedStyle being expensive. Going to close this now as we migrate to a more forward thinking uberstyle check.

Note: See TracTickets for help on using tickets.