@flamisz I think for that issue, based on a slack discussion, we were going to use the old API by default and switch to the new one only if the old one wasn't available to support older browsers that may not support the new API (eg, on mobile devices).

@flamisz as suggested in #17259 (comment) wondering if it makes sense to prefer reading data from the older API and only fallback to the newer API if the older doesn't exist anymore for reasons mentioned in the comment and also in case eg some browsers now have not responseEnd but domLoading etc. It seems like a safer way.

Additionally would need to check if performanceAlias.timing API has responseEnd or if a different property needs to be used there (domLoading).

Also to fully fix the issue need to add the rounding to integer values (see eg #17259 (comment) and #17259 (comment) as the newer API returns floats.).

And we'd also need to check it eg with iPad running iOS9.3 with Safari 9 if any possible to see if the newer API behaves faulty there maybe as we say in our analytics some funny numbers there (we have browserstack account for this)

@tsteur and @diosmosis thanks for the review.
The performance.timing API contains responseEnd, we already use it for another data.

I can make the old one default and round them integer (interestingly Firefox uses integer, but Chrome does not).

I thought now we know what is the main issue, we'd like to use the new API because we talked about using the old one by default before @sgiehl found the problem, that's why I didn't change that part, but I'll do it now.

The rounding was already solved in this PR #17372

build js

build js

@flamisz looks like the JS tests aren't passing (JSLint failing), can you take a look? Would be ready to merge afterwards.

jslint error:

if ('domLoading' in performanceData)
Unexpected 'in'. Compare with undefined, or use the hasOwnProperty method instead.

@flamisz is it possible to disable jslint for that line?

@flamisz is it possible to disable jslint for that line?

@diosmosis I can use undefined (newest commit) or try to disable for that line, but this is from the jslint doc:

JSLint was designed to reject code that some would consider to be perfectly fine...
...JSLint will hurt your feelings. Side effects may include headache, irritability, ...

😄

build js

@flamisz just had a thought, I'm not sure what valid values are acceptable for this code, but are null values accepted here or should we also check for them? (I know the previous code would have let them pass, just thought about it now.)

btw could maybe just use hasOwnProperty or so instead of ignoring jslint there maybe?

@flamisz just had a thought, I'm not sure what valid values are acceptable for this code, but are null values accepted here or should we also check for them? (I know the previous code would have let them pass, just thought about it now.)

@diosmosis it returns a unsigned long long, it can be 0 but not null.

btw could maybe just use hasOwnProperty or so instead of ignoring jslint there maybe?

@tsteur I used the undefined check instead, because the hasOwnProperty tricky sometimes, if it's an inherited property. The undefined should work the same way as the in, because the value itself can't be undefined, only unsigned long long.

@tsteur can you look at this comment quickly: #17427 (comment) ?

@diosmosis Might be good to test this in IE10 or so (seems pretty much the oldest browser that supports this API see https://caniuse.com/?search=performance ). Not sure if it could potentially result in an error if it wasn't defined etc. I think usually we'd maybe do typeof aaa.bbb !== 'undefined' (the result of typeof would need to be assigned to a variable first). Seems in past we usually used isDefined(aaa.bbb)? No big preference though. For consistency might be better to use isDefined().

Hi @flamisz, when you're able to can you take a look at #17427 (comment)? (making this comment since you weren't pinged in thomas' comment)

build js

@diosmosis and @tsteur I'm using isDefined() now. I haven't seen before we have this function. Make sense to use it.