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

Option to stop Modernizr converting "no-js" to "js" #1385

Closed
triblondon opened this Issue Jul 4, 2014 · 6 comments

Comments

Projects
None yet
2 participants
@triblondon
Contributor

triblondon commented Jul 4, 2014

I would like to propose a new option to govern this default behaviour. Perhaps:

ModernizrProto._config['jsSupportClass'] = true | false | ["from", "to"];

If true (default behaviour for backwards compatibility) it replaces no-js with js. If false, it doesn't do anything. If an array, it replaces the first value with the second.

Use case: A site (eg ft.com) chooses to set the bar for JavaScript execution higher than the simple presence of some kind of JavaScript engine. This approach has been popularised by the BBC and was named Cuts the mustard by their dev team. UAs that do not 'cut the mustard' should be treated as if they have no JS capability, thereby reducing testing overhead and easing support of legacy clients.

This will increasingly become more of an issue as the performance gap between ES6-supporting UAs and those that support only ES3 challenges developers to write increasingly bizarre code to take advantage of ES6 features while somehow continuing to provide some capability for legacy clients.

We also wish to support a more complex use case in which there are multiple cuts the mustard tests - eg one for dynamic advertising and analytics, and a higher one for everything else.

If there's agreement from maintainers on an approach I'm happy to code it up and submit a PR.

Cheers.

@triblondon triblondon changed the title from Option to stop Modernizr converting "js" to "no-js" to Option to stop Modernizr converting "no-js" to "js" Jul 4, 2014

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Jul 5, 2014

Is there a reason you need to hook into the js/no-js, as opposed to mustard/no-mustard test that is defined elsewhere?

Im open to the idea of it being configurable in some way, however "no javascript" has a pretty clear common meaning (at least in my eyes).

@triblondon

This comment has been minimized.

Contributor

triblondon commented Jul 5, 2014

You might conceivably choose to only run Modernizr if your CTM test passes, in which case js/no-js defy the 'clear common meaning' (though I think it's still correct labelling - it says whether JS is being run). And if you run Modernizr regardless of whether the CTM test passes (in which case I'd worry slightly about what clients might be tripped up by Modernizr itself), you get js/no-js labels that are unhelpful and misleading to developers that might be new to the project.

What is currently intended by this behaviour is "This browser supports JavaScript" but that's actually a derivative of the real logic, which is "Modernizr has run successfully". I'm suggesting that some developers might want to derive a different meaning from the fact that Modernizr has run, and label it appropriately.

However, for our purposes I'd also be perfectly happy with a simple on/off without the ability to supply a custom label.

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Jul 15, 2014

reasonable enough.

would an opt-out toggle around here that would just not touch no-js/js be sufficient?

something like

if (!Modernizr.jsOptOut) {
  var reJS = new RegExp('(^|\\s)'+classPrefix+'no-js(\\s|$)');                                                                                                                                                                                                                 
  className = className.replace(reJS, '$1'+classPrefix+'js$2')
}
@triblondon

This comment has been minimized.

Contributor

triblondon commented Jul 16, 2014

Sure. Not sure I'd call it jsOptOut (negatively oriented? Maybe 'enableJSClass', default true?), but in principle that'll work.

@patrickkettner

This comment has been minimized.

Member

patrickkettner commented Jan 13, 2015

@triblondon - this is in master now, thanks @rose!

@triblondon

This comment has been minimized.

Contributor

triblondon commented Jan 14, 2015

Excellent, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment