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

feature-detect for Document Fragments #641

Merged
merged 1 commit into from
Sep 5, 2013
Merged

feature-detect for Document Fragments #641

merged 1 commit into from
Sep 5, 2013

Conversation

jokeyrhyme
Copy link
Contributor

reports true as expected in IE 6-10, Chrome, Opera, probably in Firefox
reports false on a few old Nokia handsets in the office, probably on old Blackberries too

As using Document Fragments is considered a performance and stability best-practice for DOM manipulation, I think it's worth knowing when you can actually use this technique safely.

I have probably been a little too pedantic, so do let me know if some of the tests ought to be trimmed.

@paulirish
Copy link
Member

Nice!

reading https://github.com/Modernizr/Modernizr/wiki/How-We-Test ... your tests are probably too exhaustive..

just checking !!document.createDocumentFragment may just be enough??

@jokeyrhyme
Copy link
Contributor Author

Oh, you're probably right about simplifying the tests.

Whilst researching this feature, I came across notes that IE6 does not create a Fragment, but rather creates a new Document object. Much of my code is there to confirm that whatever is returned actually behaves the way it needs to. Even though IE6 was a little strange, it does complete the test.

I just wasn't sure if we'd be this lucky with every barely-conforming browser, but I'm happy to reduce this to a simple presence check if you like.

@paulirish
Copy link
Member

Ah. Sounds like a bug in IE6 that we'd want to discover and then discard
its "support". Yeah?

@jokeyrhyme
Copy link
Contributor Author

IE6's implementation does function as expected.

Hmm, just took a look at http://www.quirksmode.org/m/w3c_core.html#t112
Looks like a completely trusted result on this involves confirming that an element can start out life in the Document, be moved to the Fragment, and winds up in the Document again after the append. I personally use Fragment only for new elements, but I can't decide if this is something that is within the "90% use case".
The DOM manipulation here would definitely slow my test down even further. :S
It also looks like it was a DOM 1 feature, oops!

I don't think we need to check some of the other quirks IE 6 and 7 (http://reference.sitepoint.com/javascript/DocumentFragment) as that falls outside the "90% use case", or outside of mine, anyhow. :P Checking if Fragments have the "normalize" methods smells like unnecessary conformance testing to me.

So, do I make this more expensive to cover a known broken implementation? Or do I slim it down even more and trust that most people will never use the broken part?

@sindresorhus
Copy link
Contributor

I would say; slim it down!

@paulirish what do you think?

@jokeyrhyme
Copy link
Contributor Author

@paulirish @sindresorhus I've slimmed this down to just detecting that the methods required to use Document Fragments have been defined. Further, I've updated this for compatibility with Modernizr 3.

Please let me know if there are any further outstanding issues. <3

@paulirish
Copy link
Member

nice! thanks for the update!

On Tue, Apr 2, 2013 at 10:35 PM, Ron Waldon notifications@github.comwrote:

@paulirish https://github.com/paulirish @sindresorhushttps://github.com/sindresorhusI've slimmed this down to just detecting that the methods required to use
Document Fragments have been defined. Further, I've updated this for
compatibility with Modernizr 3.

Please let me know if there are any further outstanding issues. <3


Reply to this email directly or view it on GitHubhttps://github.com//pull/641#issuecomment-15819236
.

@patrickkettner
Copy link
Member

@jokeyrhyme it would be great ot get this merged in - could you squash all of the commits down into a single one?

@jokeyrhyme
Copy link
Contributor Author

@patrickkettner done!

@patrickkettner
Copy link
Member

tiny nit, then 👍

reports true as expected in IE 6-10, Chrome, Opera
@jokeyrhyme
Copy link
Contributor Author

@patrickkettner done!

@stucox
Copy link
Member

stucox commented Sep 5, 2013

LGTM, cheers Ron :-)

stucox pushed a commit that referenced this pull request Sep 5, 2013
feature-detect for Document Fragments
@stucox stucox merged commit 602f78b into Modernizr:master Sep 5, 2013
patrickkettner pushed a commit to patrickkettner/Modernizr that referenced this pull request Feb 22, 2015
feature-detect for Document Fragments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants