Use Node.contains to check if a document contains a node #8637
Conversation
|
It works since IE5 :) |
dhoko
force-pushed
the
feat/nodeContains
branch
from
fcfa15b
to
6f234ba
Compare
|
Hi @dhoko - thanks for the pull request. I noticed that the JavaScript tests are failing: https://travis-ci.org/piwik/piwik/jobs/79610460 could you take a look and try to fix the build? Cheers! |
dhoko
force-pushed
the
feat/nodeContains
branch
from
6f234ba
to
faeaaf8
Compare
|
Fixed, tests are ok with both Phantomjs and Chrome, |
|
But the test suite is buggy, sometimes it doesn't work sometimes it works. Ex 5min ago:
I |
| element = element.parentNode; | ||
| } | ||
| return false; | ||
| return documentAlias.documentElement.contains(element); |
There was a problem hiding this comment.
By which browsers it is supported? We need to support down to IE6 ideally older and all kinda stuff :)
There was a problem hiding this comment.
But we need to validate it with e2e tests. (cannot run a VM :/)
There was a problem hiding this comment.
Thx! It's also supported since Firefox 9.0. @mattab do we need to support older versions? I'm not sure re our policy here.
There was a problem hiding this comment.
IE5 sounds good, even IE6 support and greater is/ would be enough for JavaScript tracker
There was a problem hiding this comment.
I meant re Firefox 9+. 2012 there were still > 2% using it, I think 2015 it's still > 1.5% according to http://www.w3schools.com/browsers/browsers_firefox.asp . I personally do not see a problem there but just wanna be sure. Maybe we should mention somewhere which versions we try to support as we often had this topic
Does not exist for FF <= 9
|
Hi @dhoko maybe we could merge this PR in Piwik 3.0.0? |
mattab
added
the
Pull Request WIP
|
I wanted to upgrade this PR, but since the beginning I have this #9492 It's hard to work with it :/ PS: Yes copy/paste this message ➿ |
|
Moved to 3.0.0-b2 - @tsteur maybe you could review/maybe-merge this one? |
|
@dhoko if you are still interested in this can you issue PR again against 3.x-dev branch? 2.X is in LTS mode so we don't merge any possibly breaking changes |
Cf Node.contains