Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Showing element with jQuery does not update display style #994

Closed
jamescookie opened this issue Jan 9, 2015 · 6 comments
Closed

Showing element with jQuery does not update display style #994

jamescookie opened this issue Jan 9, 2015 · 6 comments

Comments

@jamescookie
Copy link

I cannot 'show' an element with jQuery?

it('should show elements', function() {
    document.body.innerHTML = '<style>.one {display: none;} </style><div class="one"><div class="inner">hello</div></div>';

    expect($('div.one').css('display')).to.equal('none');
    $('.one').show();
    expect($('div.one').css('display')).to.equal('block'); // fails, reports 'none'
});
@Sebmaster
Copy link
Member

This seems to be because we do not have default stylesheets which mark div elements as display: block for example. When using show() jQuery tries to determine the correct default display value for that tag name (which is empty in our case and block in the browsers).

@Sebmaster
Copy link
Member

Simplified test case (for jsdom)

"use strict";

var jsdom = require("../..");

exports["div should have default element stylesheet"] = function (t) {
  jsdom.env('<script src="file:///' + __dirname + '/files/jquery-2.1.3.js"></script>',
    {
      features: {
        ProcessExternalResources: ["script"]
      },

      done: function (err, window) {
        t.equal(window.$('<div>').appendTo('body').css('display'), 'block');

        t.done();
      }
    });
};

In comparsion to a browser:

<!doctype html>
<html>
<head>
    <script src="http://code.jquery.com/jquery-2.1.3.min.js"></script>
</head>
<body>
    <script>
        console.log($('<div>').appendTo('body').css('display'), 'should be block');
    </script>
</body>
</html>

appendTo('body') is very important in this test case, without it the default style sheets are not applied (which also makes an interesting test case the other way around)!

@domenic
Copy link
Member

domenic commented Jan 9, 2015

Wow, nice find @Sebmaster in root-causing the bugs in play here.

I wonder how we can do this. There will presumably be some trickery involved in making cssstyle aware of default style sheets, without having to insert them into the document (which causes observably different results).

See also domenic/html-as-custom-elements#27

@akhaku
Copy link
Contributor

akhaku commented May 7, 2015

Ran into this issue today myself. How about adding a style element to the head of the generated document, which has a set of rules that emulate the default style sheet used by common implementations? What do you think?

@domenic
Copy link
Member

domenic commented May 7, 2015

No, that would break any code that looks at e.g. document.styleSheets[0] (or document.styleSheets in general, really).

Someone needs to test whether default stylesheets affect the results of getPropertyValue (in which case we should introduce this into cssstyle) or whether they're handled at a higher level and only show up in getComputedStyle (in which case we'll need to handle this in jsdom).

@akhaku
Copy link
Contributor

akhaku commented May 7, 2015

Thanks for clarifying, sorry for my clueless response above. Tried it out in chrome, safari and FF on OSX

var a = document.getElementById('foo')
window.getComputedStyle(a).getPropertyValue('display')
>> "block"
a.style.getPropertyValue('display')
>> null

so it looks like it shows up in getComputedStyle. I'll pull down the source and mess around with it for a bit, will send up a pull request if I can get it working. Thanks!

akhaku added a commit to akhaku/jsdom that referenced this issue May 8, 2015
@akhaku
Load default html.css stylesheet. Fix jsdom#994.
2b2e27b
@akhaku akhaku mentioned this issue May 8, 2015
Closed
akhaku added a commit to akhaku/jsdom that referenced this issue May 8, 2015
@akhaku
Load default html.css stylesheet. Fix jsdom#994.
d520cd9
akhaku added a commit to akhaku/jsdom that referenced this issue May 8, 2015
@akhaku
Load default html.css stylesheet. Fix jsdom#994.
63ce99b 
Also simplify some getComputedStyle tests while there.
akhaku added a commit to akhaku/jsdom that referenced this issue May 12, 2015
@akhaku
Apply default styles when getting computed styles
f69af97 
Fixes jsdom#994.

Also simplify some getComputedStyle tests while there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Sebmaster @akhaku @domenic @jamescookie