Bug Tracker

Opened 13 years ago

Closed 13 years ago

#1661 closed bug (fixed)

curCSS does not validate returned value of getComputedStyle

Reported by: lmnt Owned by: davidserduke
Priority: major Milestone: 1.2.2
Component: core Version: 1.2.1
Keywords: Cc:
Blocked by: Blocking:

Description

in line 729 of jquery 1.2.1:

ret = prop == "display" && swap[stack.length-1] != null ?
    "none" :
    document.defaultView.getComputedStyle(elem,null).getPropertyValue(prop) || "";

this could probably be done more elegantly but for my purposes it's suited.

will fail if getComputedStyle() does not return as expected (such as when working with iframes). This is easily remedied by adding a simple check:

ret = prop == "display" && swap[stack.length-1] != null ?
    "none" :
    (document.defaultView.getComputedStyle(elem,null) ? 
        document.defaultView.getComputedStyle(elem,null).getPropertyValue(prop) || "" : "" 
    );

Change History (7)

comment:1 Changed 13 years ago by danielnashnz

You got there before me! Shouldn't the code be:

ret = (prop == "display" && swap[stack.length-1] != null) ? "none" : (document.defaultView.getComputedStyle(elem,null) != null) ? document.defaultView.getComputedStyle(elem,null).getPropertyValue(prop) : "";

??

comment:2 Changed 13 years ago by mc_funkster

I vote for this bug to get fixed (soon) with danielnashnz' fix.

Imnt is missing a ')' after the check for a null computed style

comment:3 Changed 13 years ago by geoffreyk

I just ran headlong into this bug as well with an iframe. Seems that the fix is simple and localized. Just trying to pump this bug up a bit. (I hate to run with a patched version of jquery)

comment:4 Changed 13 years ago by davidserduke

This appears like it could be the same as #1779 which was fixed.

comment:5 Changed 13 years ago by geoffreyk

I don't think so. I just applied the changes from 1779, and that doesn't fix this problem.

this code does seem to fix the problem, though I don't know how this would affect #1779

				ret = (prop == "display" && swap[stack.length-1] != null) ? 
					"none" : 
					(document.defaultView.getComputedStyle(elem,null) != null) ? 
						document.defaultView.getComputedStyle(elem,null).getPropertyValue(prop) : "";

comment:6 Changed 13 years ago by davidserduke

need: CommitTest Case
Owner: set to davidserduke
Status: newassigned

Do you have a test case I could use that displays the problem?

comment:7 Changed 13 years ago by davidserduke

Resolution: fixed
Status: assignedclosed

If you applied the patch to 1.2.1 then it definitely won't work. Some other changes have been made to the function in SVN that are required.

I think this is fixed in SVN and am going to mark it as such. If you have a test case that doesn't work please attach it and reopen the bug.

So fixed in [3644].

Note: See TracTickets for help on using tickets.