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.

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

Traversing: $.fn.contents() support for object #4046

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
2 participants
@emibloque
Copy link
Contributor

commented Apr 18, 2018

Fixes gh-4045

Summary

As described in gh-4045, this PR allows <object> document content to be accessed again using contents() function.

Checklist

@@ -145,7 +145,7 @@ jQuery.each( {
return siblings( elem.firstChild );
},
contents: function( elem ) {
if ( nodeName( elem, "iframe" ) ) {
if ( nodeName( elem, "iframe" ) || nodeName( elem, "object" ) ) {

This comment has been minimized.

Copy link
@dmethvin

dmethvin Apr 18, 2018

Member

I wonder if we should just be checking typeof elem.contentDocument !== "undefined" or something to that effect. The check for a specific element is how the regression occurred. Are there any other elements where this might apply?

This comment has been minimized.

Copy link
@dmethvin

dmethvin Apr 18, 2018

Member

Looks like it's applet, frame, iframe, and object.

This comment has been minimized.

Copy link
@emibloque

emibloque Apr 19, 2018

Author Contributor

Good point, thanks!
I'll use that check and work on tests for those tags too.

Also, I'm seeing that the test for Node 8 is failing because the route for the svg is returning a 404.
Any advice on best practices for avoiding this?

This comment has been minimized.

Copy link
@dmethvin

dmethvin Apr 20, 2018

Member

Is there a specific reason why you're using baseURL in the test? Usually a relative reference like data/1x1.svg should work.

This comment has been minimized.

Copy link
@emibloque

emibloque Apr 25, 2018

Author Contributor

I followed how it was done in other tests. Actually the problem was that I didn't add the svg extension to the karma config 😅.

I just applied the change you mention. However, due to the lack of browser support for applet nowadays, I wasn't able to prepare a test for this tag.

@emibloque emibloque force-pushed the emibloque:gh-4045-object-contents branch from bd2c5a3 to 262417e Apr 25, 2018

@emibloque

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2018

Hi @dmethvin !
Just to follow up on this, could I help with anything else here?
If there is anything left to do I will gladly try to work on that 😃

@dmethvin

This comment has been minimized.

Copy link
Member

commented May 10, 2018

@emibloque Nope, we're good. It should land before the next release goes out but I'm not sure when that will be at the moment. Thanks for the contribution!

@dmethvin dmethvin closed this in 0ba8e38 May 14, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Nov 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.