arielsalminen / feature.js Goto Github PK

The localStorage check is ambiguous as it includes unsupported as well as out of quota in normal browsing and the Safari "I'll fail you if you're in private browsing mode and try .setItem" tests in one. It will also pass a false positive if you try that same thing on Android/Chrome; you can set and remove all you like without error but nothing actually happened.

A better check would be to verify localStorage exists on window followed by whether you can successfully write and read in a try ... catch block as .removeItem returns undefined regardless of if the key existed or not.

how can I check the websocket support? Is there going to add this feature? or there is some way I can easy to check?

Currently, you have this code in the 'pfx' function:

var ucProp = prop.charAt(0).toUpperCase() + prop.substr(1),
  props = (prop + " " + prefixes.join(ucProp + " ") + ucProp).split(" ");
  memory[prop] = null;

Shouldn't the second line end with a comma, instead of a semi-colon?
Either that, or the indentation on line three is incorrect.

I would expect the indentation is off, since you're declaring the 'memory' Object a few line higher up in the 'pfx' function.

Safari does now support srcset and sizes, but for some reason sizes is not available on the img object, and so your test reports that Safari doesn't have support.

(I really want to be able to set sizes on some dynamically added content, so if anyone knows if sizes is available through some other means please let me know.)

On lines 221-228, what's the purpose of the expose function? Seems like setting the global variable directly would give the same result with less code and no drawbacks that I can see.

window.feature = new Feature();

In fact, the minifier seems to be doing that anyway at the end of line 6 b.feature=new g.

First off, great library

Although supporting isomorphic Javascript doesn't seem necessary at all, it would be nice if there was a conditional check for the window element. When doing server side rendering, an exception is throw when pulling in feature.js from node

}(window, document));
  ^

ReferenceError: window is not defined

Perhaps an empty object could be returned.

in few cases there is an appropriate check for window.navigator however in the case of 'geolocation' there is an explicit binding for navigator object which is not necessary exist in the closure. wrap it out or add an extra step to check in the util

Hello Viljamis,

How about using the following wrapper for FeatureJS so that it can be loaded as an AMD module?

(function (global, document,factory) {
    if (typeof define === 'function' && define.amd) {
        // AMD. Register as an anonymous module.
        define(factory);
    } else {
        // Browser globals
        global.feature = factory();
    }
}(window, document, function () {
    // ... Fearure JS code goes here...
    return feature;
}));

Hi,

Why we shouldn't have a build system to build automatic minified version. I think we really need this. we have 6 pull requests and they didn't minify their js file.

I recommend to use Gulp or Grunt.

I like to fix this issue too.

thanks!

I think Feature function constructor is useless, we should use the Feature Object directly. It can improve the performance of Feature
a little of code:

Feature = {
    css3Dtransform : (function() {
      var test = (!util.old && util.pfx("perspective") !== null);
      return !!test;
    })()
}

and Why should we have the constructor that does nothing?

and I'm interested in fixing this issue.

There is v1.0.1 tag on GitHub but missing on npm

The ContextMenu test requires both contextmenu and HTMLMenuItemElement to be present. As the latter is currently unsupported in everything but Gecko this test returns false across all other major browsers. More importantly contextmenu alone is enough to override a context menu (or 'right-click' as is mentioned on the test page).

Is HTMLMenuItemElement necessary, and if so should the test be split in two?

The following code suppresses a context menu on all modern browsers:
document.addEventListener( "contextmenu", function(e) { e.preventDefault(); });

One of our clients noticed the webgl check failing in their Chrome. We reproduced this in BrowserStack on Windows7-Chrome55.

The problem appears to be that the browser does not set up the 'webgl' context on canvas elements that are not in the DOM. AKA the following test doesn't work:

 // Test if WebGL is supported
webGL : (function(el) {
  try {
    return !!(window.WebGLRenderingContext && (el.getContext("webgl") || el.getContext("experimental-webgl")));
  } catch(err) {
    return false;
  }
})(util.create("canvas")),

If I do the following, it works fine (add the canvas to the actual DOM before checking).

 // Test if WebGL is supported
webGL : (function(el) {
  try {
    document.body.appendChild(el);
    var check = !!(window.WebGLRenderingContext && (el.getContext("webgl") || el.getContext("experimental-webgl")));
    document.body.removeChild(el);
    return check;
  } catch(err) {
    return false;
  }
})(util.create("canvas")),

In the "old android" test the string "Android" might be "android" -can't recall which devices this occurred on, but a simple (A|a)ndroid should check for both. Great work btw👍

In a future version of Chrome the navigator.userAgent and navigator.appVersion and navigator.platform API will be deprecated.

As of today (Chrome 92.0.4515.159), a message will show under the Issues tab

Reference: https://blog.chromium.org/2021/05/update-on-user-agent-string-reduction.html

canvas : function () { return !!window.HTMLCanvasElement; },

It's tons faster since it doesn't require the overhead of creating an element.

This also works:

canvas : function () { return !!window.CanvasRenderingContext2D; },

See http://stackoverflow.com/questions/2745432/best-way-to-detect-that-html5-canvas-is-not-supported

The key used for the localStorage test is "x", and the test removes it from localStorage.

If a site or application already uses this key (which is not that unlikely, e.g. if a not very clever script uses "x" and "y" without any prefix to store some coordinates), the site/app's data will be deleted.

I suggest using var test = "featurejs-test"; or something shorter but less likely to create conflicts.
One short option is to use var test = Date().

Since these are simple object properties and the feature detection will fail silently if you use the wrong name, I'd suggest keeping the feature names as simple as possible.

For instance, why css3Dtransform and not css3DTransform?

feature.async
feature.addeventlistener
feature.canvas
feature.classlist
feature.cors
feature.contextmenu
feature.css3dtransform
feature.csstransform
feature.csstransition
feature.defer
feature.devicemotion
feature.deviceorientation
feature.geolocation
feature.historyapi
feature.placeholder
feature.localstorage
feature.matchmedia
feature.pictureelement
feature.queryselectorall
feature.remunit
feature.serviceworker
feature.sizes
feature.srcset
feature.svg
feature.touch
feature.viewportunit
feature.webgl

I feel as though it would be a good idea to break the tests up into modules which offers several benefits:

• Custom builds.
• Maintainability.
• Importing only the tests you need.

Is there any chance PWA features can be detected and exposed?

https://web.dev/learn/pwa/detection/

For instance, some browsers support PWA install prompts, some other support installing the PWA but manually (no prompt) and others don't even support install at all.

That sounds like a good use case for feature.js

I know the file is small and should be fast, perhaps I'm micro optimizing but Instead of running all the code right away would using getters make it faster, therefore only when each feature is checked:

if(feature.webGL) { // right here it'll run the getter instead of onload

}

Perhaps only do this if this file exceeds a certain file size?

First off, great library

Although supporting isomorphic Javascript doesn't seem necessary at all, it would be nice if there was a conditional check for the window element. When doing server side rendering, an exception is throw when pulling in feature.js from node

}(window, document));
  ^

ReferenceError: window is not defined

Perhaps an empty object could be returned.

This project doesn't have a software license. I'd love to have one so I can use this library!

Perhaps all it needs is to be condensed and maybe written in a should prose style, ala normalize

Hi,

It can be faster, feature.js should work as lazy library. every property should be called when I looked it up. not all of them on load time.

I highly recommend you to read getter in object MDN

and I'm interested in fixing this issue.

For example UC Browser in Android 2.3 which supports css transform
I don't have any Android 2.x device but I guess feature.js may not work right in that situation.

Thx

Current implementation might give false positive on some older Android stock WebKits in very rare cases. This issue is currently being tracked here in case the feature detection needs changes.

Instead of

create : function (el) { return document.createElement(el); },

use

create : document.createElement.bind(document),

I infer that you're not doing IE8 support, and everything else supports Function#bind. It's kind of a performance enhancement, but maybe not worth it.

When running yarn or npm install for the first time we're hit with:

warning gulp > [email protected]: gulp-util is deprecated - replace it, following the guidelines at https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5
warning gulp > vinyl-fs > [email protected]: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.js
warning gulp > vinyl-fs > glob-stream > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning gulp > vinyl-fs > glob-stream > glob > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning gulp > vinyl-fs > graceful-fs > [email protected]: This module relies on Node.js's internals and will break at some point. Do not use it, and update to [email protected].
warning gulp > vinyl-fs > glob-watcher > gaze > globule > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning gulp > vinyl-fs > glob-watcher > gaze > globule > glob > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning gulp > vinyl-fs > glob-watcher > gaze > globule > glob > [email protected]: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.    js  
warning gulp-uglify > [email protected]: gulp-util is deprecated - replace it, following the guidelines at https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5
warning jest > jest-cli > jest-config > jest-environment-jsdom > jsdom > [email protected]: use String.prototype.padStart()

I found feature.touch is true in IE11, is it right?

When running yarn or npm install for the first time we're hit with:

warning gulp > [email protected]: gulp-util is deprecated - replace it, following the guidelines at https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5
warning gulp > vinyl-fs > [email protected]: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.js
warning gulp > vinyl-fs > glob-stream > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning gulp > vinyl-fs > glob-stream > glob > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning gulp > vinyl-fs > graceful-fs > [email protected]: This module relies on Node.js's internals and will break at some point. Do not use it, and update to [email protected].
warning gulp > vinyl-fs > glob-watcher > gaze > globule > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning gulp > vinyl-fs > glob-watcher > gaze > globule > glob > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning gulp > vinyl-fs > glob-watcher > gaze > globule > glob > [email protected]: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.    js  
warning gulp-uglify > [email protected]: gulp-util is deprecated - replace it, following the guidelines at https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5
warning jest > jest-cli > jest-config > jest-environment-jsdom > jsdom > [email protected]: use String.prototype.padStart()

So depending on whether the plugin exists or not, we know if we are connected, have webGL etc.

If not, I wouldn't mind becoming the maintainer.

Hi, Whats the browser support like?

This project is very interesting. It would be good to have it as a bower package.

Thanks.

When the user has blocked cookies and data in chrome, this error occurs:
Failed to read the 'localStorage' property from 'Window': Access is denied for this document.

This is a result of trying to read window.localStorage.length in feature.js line 150.
An error is thrown whenever the localStorage property on window is accessed.

I have a problem that it seems that Edge dos not support element.scrollIntoView if element is an node.

It appears that scrollIntoView can only be used by some elments. http://www.java2s.com/Code/JavaScriptReference/Javascript-Methods/scrollIntoViewisappliedto.htm But this reference is a bit weak and I cannot find this information in other specs.

Nevertheless is it worth/possible to test this by featurejs?

Feature detection largely came about because checking for browser versions via the user agent string is unreliable. This is for multiple reasons including (as I'm sure you're already aware):

1. false positives (e.g. Opera and Edge include "chrome" in their user agent string)
2. Spoofing
3. Assuming that a certain User Agent is a reliable means of testing things (e.g. some browsers such as UC Browser and Dolphin Browser were available on very old versions of Android, but supported features the default Android browser of the time did not. So checking for that Android version would have been unreliable)

So including it as part of some of your checks seems somewhat contradictory.