In order to embrace modern web technologies we need to be able to provide fall backs for older browsers.

All of these, like the HTML5shiv, are Javascript based. We need a way to conditionally load Javascript files based on feature detection.

Related issues

#1033392: Script loader support in core (LABjs etc.)

Use cases

#1261002: Draggable tables do not work on touch screen devices
#1260800: Kill the overlay for widths below 640 pixels
#1170478: Responsive Images
#1137800: Increase minimum size of targets for touch screens
#1277352: Responsive vertical tabs
#1192068: Graceful degradation for browsers that don't support media queries
#1512194: Use HTML5 datalists for autocomplete
#1168246: Freedom For Fieldsets! Long Live The DETAILS.
#1137920: Fix toolbar on small screen sizes and redesign toolbar for desktop

ISSUE SUMMARY:

Problem/Motivation

Right now in D7 all feature detection is hard-coded in specific JS files. Proposed resolution

Include Modernizr in core. What parts of Modernizr we choose to include is important. If we go too lightweight then we run the risk of hitting a brick wall because of inability to predict use cases. If we go too heavy then it adds way too much redundant overhead to page loads.

The proposal is to modularise Modernizr within Drupal. Recreating the functionality of the Modernizr production builder

If we want to fully embrace HTML5 and Responsive Design in Drupal 8 then we need Modernizr in core.

Remaining tasks

Create patch.

API changes

Modules and themes would be able to call hook_device_context() in order to load the specific parts of Modernizr that they require.

Original report by bayousoft

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

rupl’s picture

Issue tags: +html5, +Front end

Some of the possibilities discussed in a recent #drupal-html5 meeting:

Within the scope of D8 HTML5 initiative we've only identified two needs so far:

Damien Tournoud’s picture

Status: Active » Closed (duplicate)
Damien Tournoud’s picture

About the loading issue specifically related to IE, see #865536: drupal_add_js() is missing the 'browsers' option.

alanburke’s picture

Status: Closed (duplicate) » Active

The HTML5 shiv is just the tip of the iceberg.
In order to to meaningful modern web development, we need the ability to to do client side feature detection.
The best way to do that is with Modernizr - rather than writing this ourselves on a per use or per project basis.

#1077878: Add HTML5shiv to core
This can be handled by Modernizr by itself, but we can come back to reworking that implementation if we can agree on adding Modernizr.

Asynchronous script loading can also be handled using Modernizr.
#865336: Feeds XPath Parser: Import feedback required
drupal_add_js could be completely reworked if Modernizr was available.
Instead of adding scripts using conditional comments - which is almost as bad as browser sniffing - we can load scripts based on client-side capabilities.

So as well as these two issues,
we can also use it to load polyfills for other HTML5 features such as the forms placeholder attribute.

alanburke’s picture

Jacine’s picture

I really don't like the idea of rolling our own, when something out there as good as Modernizr exists.

There are some issues with Modernizr, though. It's MIT & BSD licensed and from what I've heard it may not be possible to change this. It's also recommended to customize the build, based on what parts are needed. Deciding which parts we really need will be tough. Even if we were to just include the basics like the Shiv/CSS classes/Modernizr.load, it would absolutely need to work like jQuery UI does with hook_libraries() somehow, or it would be a nightmare for contrib and dealing with dependencies. That could end up being a major pain, but we definitely can't just include the full build either.

I've been thinking about this a lot, and I don't know what's best, but I think we need to discuss it and figure something out soon.

LewisNyman’s picture

Issue tags: +mobile, +d8mux

Hey Jacine,

Thanks for the weight in, I can address some of your issues with Modernizr.

Legal
Modernizr is licensed under the MIT licence, which is not copyleft. It allows third parties (us) to republish under any licence we choose to, including GPL. I've had confirmation on this personally from Paul Irish.

Technical
What parts of Moderizr we choose to include is important. If we go too lightweight then we run the risk of hitting a brick wall because of inability to predict use cases. If we go too heavy then it adds way to much redundant overhead to page loads.

So what I am proposing is that we modularise Modernizr within Drupal. Recreating the functionality of theModerizr production builder

Modules and themes would be able to call hook_device_context() in order to load the specific parts of Modernizr that they require.

I can provide many other use cases were we will need this in core. If we want to fully embrace HTML5 and Responsive Design in Drupal Eight then we need Modernizr in core.

Jacine’s picture

Just so it's clear this is not "Jacine the HTML5 Lead" speaking here, it's just "Jacine the core contributor." ;)

I am no expert on licensing, so I will happily defer that to someone else, but if it wasn't a problem, I wonder why the Modernizr Drupal contrib module doesn't include the code directly instead of making users download it separately.

Modules and themes would be able to call hook_device_context() in order to load the specific parts of Modernizr that they require.

Why do we need a hook_device_context()? What's wrong with Drupal's hook_libraries() and drupal_add_library()? This works well for jQuery UI. Splitting it up would definitely need to happen, like I said above, but it doesn't appear straightforward at all. Looking at the source code, it appears that the code is in a few separate repositories (like. yepnope.js and respond.js), and the build script they use on modernizr.com appears to be specific to that site. We would also need something that makes the build work server-side, not client-side, and this could end up being a real pain to maintain. With jQuery UI, this problem doesn't exist. It's all in one repository that's structured and ready go.

I can provide many other use cases were we will need this in core. If we want to fully embrace HTML5 and Responsive Design in Drupal Eight then we need Modernizr in core.

As far as core use cases are concerned, getting a solid list of these together would be good. Including any additional library in core like this is going to get pushback. There are lots of people focused on making core more lightweight and easier to maintain right now. Personally, I'm not convinced it's all that necessary right now. Is it something I use? Sure, but not on every project and even if I did, that doesn't necessarily mean everyone else will want to. I don't believe that we NEED to include Modernizr in core to fully embrace HTML5. Including any JavaScript file is a piece of cake, and some would argue that the decision to use Modernizr or anything else like it, is a decision that should be made in the theme.

Also, has there been a conversation about dealing with media queries, where respond.js was decided upon? It's not like respond.js is the only solution out there and there's nothing stopping us from adding JavaScript files directly in the core themes right now. If would imagine that it's going to need a whole separate discussion, if there isn't already one going on, because there are a few solutions out there right now.

I'm really just trying to say that we need to really think it through and truly justify its inclusion. Ultimately, I think this will come down to being able to demonstrate that:

  • We have a number of solid use cases.
  • The community supports adding it.
  • It can be easily maintained in Drupal core.

Regarding the HTML5 Shiv/shim, yes it's in Modernizr, but it's also going into jQuery at some point, which we already have. I've talked to Paul Irish about this, and while it's not ready for primetime yet, it will be by the time D8 comes out. Once that happens the HTML5 Shiv will be removed. So basically, it's a non-issue for that.

LewisNyman’s picture

Why do we need a hook_device_context()? What's wrong with Drupal's hook_libraries() and drupal_add_library()?

hook_libraries would work great if we just wanted to drop the full build of Modernizr in. What we have is actually a more complex combination of tests and additional functionality that depends on each other. The array of dependencies can be found on Github

Splitting up each individual function is also a chore. The build script on moderizr.com is built in javascript and actually takes a full Modernizr script and chops it up. This is more maintainable then having to split the files up ourselves on every new release of Modernizr.

...some would argue that the decision to use Modernizr or anything else like it, is a decision that should be made in the theme.

There are use cases where a core module needs to be able to utilise Modernizr. The toolbar and overlay module are two examples that need to adapt based on device context and I'm not talking about CSS3 support.

Another obvious example is the additions of HTML5 form elements and attributes. Are we really going to add these options into core when they aren't supported by major browsers?

Let's take the Placeholder attributes as an example. Let's assume the developers using formAPI or a front facing GUI are new comers don't know how we've implemented this functionality. They add in the [#placeholder] attribute, it works great in their browser and then without warning they find out this functionality they have implemented does not work.

Should they add delve in through our implementation and work out how to fix it? Or should they scrap our supplied solution entirely and roll their own? This is not a good developer experience.

You can apply this scenario to
, date, range, color. They have no decent server side fall back.

Building up a list of core use cases is a good idea. Let's try and work towards that, it's more positive.

Jacine’s picture

@lewisnyman Are you planning on attending HTML5 meetings? We've discussed some of this already, and decided to address specific use cases for the new form elements in individual issues. Support for the placeholder attribute has already been committed and the consensus was that we don't need a polyfill for that specific case. I get that you are trying to build a list of core use cases and I am not trying to discourage that, I just think it would be great if you could attend and participate in the meetings so we can all get on the same page. The next one is tomorrow at 4 PM EST.

Regarding the hook_libraries() thing, what I am trying to say, and I guess it's not coming through clearly enough, is that I'm familiar with the source code and that a client-side build is likely not going to be sufficient, therefore it would be hard to maintain in core. This is debatable, of course, but even if we had a client-side build, I assume you'd be looking to do something like duplicate the http://modernizr.com/download/ page in core, which is a lot to ask for.

Anyway, it would be great to start a wiki containing potential use cases for this on g.d.o. If you want to get started on that, that would be awesome. I hope to see you at tomorrow's meeting. It would also be great to get more feedback on this from others, so I will try to do what I can to make that happen, and I'll stop playing devil's advocate and leave this to you guys to work through. ;)

gkatsanos’s picture

I don't see any reason polyfills should be in CORE. A user should have to liberty to include or not a polyfill in his theme, he shouldn't be forced to load it.

adrinux’s picture

The size of modernizr and the necessity to tailor it to a particular site have already been touched on. And in this community the tendency to think of grand engineered solutions has also evidenced itself.

I think of modernizr as largely a front end tool, and thus one that belongs in themes, probably in core themes, but still at the theme level. The suggestion of theme hooks and including fragments of modernizr seems over-engineered, a solution looking for a problem.

My vote goes with keep it simple and let each themer or site-builder add it themselves, or some other library of their choice.

taslett’s picture

Sounds like themes should have dependencies added to the info file so they can require a module such as modernizr.
My vote is to keep it out of core.

stevetweeddale’s picture

I was about clarify this issue summary and also add a lot of opinion, but there actually appears to be an issue closer to where I think we want to go - which could well scratch this itch.

To summarise what seems popular opinion in this issue and the discussion had in the html5 irc meeting yesterday: Modernizr doesn't belong in core.

However, for adding some kind of script loader (which could include conditional loading based on client-side feature tests), it turns out there is already an issue. see #1033392: Script loader support in core (LABjs etc.)

A script loader-based drupal_add_js would be super cool and could be really good for front-end performance (asynchronous loading, possible parallel execution) and if we were to push for something like yep-nope to be the solution over there, we could get client-side feature testing (and thus conditional polyfill loading) nicely baked in.

Feel free to re-open if we think that the issues should be seen as separate. However I think our best bet is in that issue, as it's broader than simply 'HTML5 polyfill-loading'; more like 'how to make Drupal JS handling more awesomer'. Which, if we play our cards right, could happily include client-side conditional loading.

stevetweeddale’s picture

Status: Active » Closed (duplicate)

edit: wierd, I thought dreditor had nuked my previous post... ignore this one.

LewisNyman’s picture

Status: Closed (duplicate) » Needs work

I'm sorry, did I miss something last night?

I thought the final action to take away was to build a list of use cases for modernizr in core?

stevetweeddale’s picture

Ah yeah, I forgot about building a list of use-cases... although it certainly seemed like the general consensus was that Modernizr wasn't going to be the best solution. Is that fair to say? My brain is pondering the list of potential use-cases for some kind of client-side script loader framework in core, which I'm planning to air over in that other issue. That's subtly but importantly different to 'reasons for Modernizr in core'.

If this issue is really about modernizr specifically, then we should probably rename it as such. Because modernizr is more than just a script loader framework - it includes one, but also bundles a load of feature tests (which could need updating/revising) and other stuff: which although nice, imply extra baggage for core.

If we're just bothered about the framework for conditionally loading JS I would suggest that the other issue could cover it, already has a few other behind it, and is older, hence why I closed this one.

stevetweeddale’s picture

Ok, so I've just read the irc log, and I'd agree that my summary was probably fairly naff. Read Jacines notes for a better one.

Though I'd still suggest this issue needs renaming. Happy with 'Add modernizr to core'?

LewisNyman’s picture

I think that's a really weighted issue title.

No one is suggesting we dump Modernizr in like we do with jQuery. What we have here are some problems and adding a conditional javascript loader, whether that is Modernizr or another library, is one of the solutions.

It is clear we don't need this kind of functionality in core for contrib modules or themes, they can rely on a contrib solution. I have started a sandbox for this. It is designed to be completely abstracted so we can update or completely swap out Modernizr without killing dependencies.

The use cases for core themes and modules are the balance for adding this kind of functionality in core. If we are going to throw Modernizr out as a solution because it is too hard then we need to think up an alternative or at least acknowledge what we are throwing away.

stevetweeddale’s picture

I totally agree with all that - a conditional loader either in contrib or core would be ace. I'm not sure we're disagreeing. Do you not think that other issue could happily address the same concerns when talking about a loader in core? If there's already some movement for a script loader in core over there, we'd do well to join that effort and push for one that can do conditional loading rather than one that just loads all JS resources asynchronously.

Modernizr includes a script loader (yep-nope), but also bundles a load of JS tests for certain features. That's the contentious bit IMHO for core. I'm not sure there are any other 'feature-test' libraries like it, which is why my suggested title was so specific.

This is the answer to your question as I [humbly!] see it: Throwing out modernizr but going for another script-loader is essentially throwing out the JS feature tests and shiv it comes bundled with, leaving that for core, contrib or custom code to provide those as needed. It reduces what would have to go into core to a slim js-loader library, and refactoring drupal_add_js to use it rather than 'browsers'.

Throwing out the whole idea entirely leaves it up to contrib to hook into drupal_alter_js and try to provide this themselves... which may or may not be easily possible.

LewisNyman’s picture

Ok, just a clarification.

When I created this issue I had no intention or opinion in loading javascript asynchronously. That's not my agenda here. What I want from Modernizr are the feature tests. These can be used as conditionals for actions far greater then just loading a JS file or not. The tests in Modernizr can be a lot more powerful then that. This issue could even be closed without including yepnope.js in core. It's just beneficial to think about both of these issues being solved by one decision. Let's not make the mistake of saying we need every component of Modernizr in core.

I'll throw up some of these use cases later tonight, I hope.

stevetweeddale’s picture

Cool, glad we're understanding one another :)

Though to be honest, including all the tests in core is probably going to be a tough sell. One that you'd need to sell to me too! Adding the framework for executing and responding to those tests, less so.

Everett Zufelt’s picture

Just to clarify, as someone at the meeting and who is initially pretty opposed to modernizr in Core I think this issue should stay open.

In he meeting (paraphrase):

1. Is there a use-case for Contrib to use modernizr? Yes.

2. Is there any reason that Modernzr cannot be added in Contrib when needed? No.

3. Is it possible to do a Modernizr.module, so that all Contrib uses are consistent and interop? Yes.

4. Does Contrib need any changes in Core to make it possible for a Contrib pluggable Modernizr.module? Nothing was suggested, but possibly the other open issue is evidence of this.

5. Are there any use-cases where Modernizr is required, or strongly desired, to accomplish something in Core? Use-cases being collected.

Once we have use-cases we can re-evaluate. If there is a problem in Core, and Modernizr can solve the problem, we need to do a cost-benefit-analysis and decide if the problem really needs solving, what the alternatives are, and how to proceed.

catch’s picture

@jacine. Drupal contrib repo has a blanket gpl-only rule. However core can ship with gpl-compatible code. Crell's the best person to run this past.

Everett Zufelt’s picture

I'm wondering if modernizr has support for sane fallback for details / summary. It would be nice to replace collapsible fieldsets w/ details / summary, but it, AFAIK, is only supported in Chrome at the moment.

Is providing a JS based fallback for d/s something that modernizr can help us do?

Jacine’s picture

@catch Thanks. We have discussed it briefly. Right now we are working under the assumption that it's not impossible, and trying to concentrate on demonstrating use cases for it, and the general level of support there is for including it in core.

@Everett, Modernizr doesn't include many specific fallbacks, but it does include tests, and yes, there is a specific test for <details> and <summary> elements. It's located here: https://github.com/Modernizr/Modernizr/blob/master/feature-detects/elem-.... It was added after the latest release, so it would be in the next version. So, if we have Modernizr + YepNope, we could use it to conditionally load a polyfill for that use case. We could also roll our own....

What I would really like, in general (in addition to gathering use cases) is for someone to break down how this could potentially work, from a technical standpoint, in a sane and maintainable way.

EDIT: BTW, there are 2 polyfills for <details> and <summary> that I am aware of per https://github.com/Modernizr/Modernizr/wiki/HTML5-Cross-Browser-Polyfills:

nod_’s picture

Status: Active » Needs work
Issue tags: +mobile, +html5, +d8mux, +Front end

Might want to work together with #1033392: Script loader support in core (LABjs etc.) and see how much overlaps.

TransitionKojo’s picture

Ok, I'll be adding/updating the Issue Summary here. Seems a bit lacking.
:-)

JohnAlbin’s picture

Title: Add a polyfill loading solution » Add a feature detection solution

Basically, we need to do this:

feature detection -> script loading -> polyfill

Since we already have #1033392: Script loader support in core (LABjs etc.) to discuss script loading, let's re-focus this issue on the first step: "feature detection". Loading up a specific polyfill for a specific feature that is lacking will be simpler if we have the other 2 bits figured out.

Amazingly, when webchick asked "where's the feature detection issue that talks about modernizr, etc" I couldn't find anything but this issue. It seems like Modernizr always came up in several issues/IRC discussions/recurring nightmares, but never got its own issue. :-)

There are several use cases for "needing a feature detected" in core. Right now in D7 all our feature detection is hard-coded in specific JS files. We can continue with that or determine a better generalized method of feature detection.

Jacine’s picture

Amazingly, when webchick asked "where's the feature detection issue that talks about modernizr, etc" I couldn't find anything but this issue. It seems like Modernizr always came up in several issues/IRC discussions/recurring nightmares, but never got its own issue. :-)

This is THE issue.

Jacine’s picture

Issue summary: View changes

Updated issue summary.

bayousoft’s picture

Issue summary: View changes

Issue Summary (bayousoft)

dodorama’s picture

I feel like we need some kind of solution implemented in core soon so that we can rely on it especially for the mobile initiative. What would be useful is to use feature detection to define the three main devices used nowadays to browse the web (desktop, tablets, smartphone). The approach described here seems interesting: http://www.html5rocks.com/en/mobile/cross-device/ where media queries and feature detection are used together to build cross-device webapp.
What would be crucial to me is the ability to serve, not just different stylesheets and scripts based on the device, but even different views. This would solve the problem of administrative tables for example. We would, then, be able to serve a different markup to a mobile device rather than try to twist the table using css. #1276908: Administrative tables are too wide for smaller screens

An overview of a hybrid approach server/client
http://www.slideshare.net/4nd3rsen/ress-responsive-design-server-side-co...
A server side script
http://www.brettjankord.com/2012/01/16/categorizr-a-modern-device-detect...

rupl’s picture

@dodorama, we're not using a server-side script. It cripples any stack that involves a reverse proxy cache, and it's ill-equipped to handle the multitude of devices connecting to Drupal. In short, it's yesterday's solution. RESS is interesting, but we need to let the Twig dust settle before we could think about doing that in a sane manner.

@lewisnyman brought up an interesting idea in the Modernizr queue, which is to create separate drupal_add_js() calls for each feature we need to detect. Then, when a test is needed, the JS code that corresponds to the test will automatically get added to a small, separate aggregate which is partitioned away from most of the other more lengthy JS code that executes after DOMContentLoaded (e.g. jQuery). We could even inline the feature detects instead of aggregating.

See Lewis' original comment here: #1030822-6: Modernizr 2 custom builder (client-side for admin UI)

gmclelland’s picture

Similar issue #1621594: Media Query Detection, but these are not the same. They can be used together.

mgifford’s picture

We need an initial patch for this issue.

Not sure if there's anything that can be pulled from the D7's
http://drupal.org/project/modernizr

It would be great if we could get this into core to help deal with the older browsers in a unified way.

EDIT: Adding some other issues where we need a unified approach to polyfill for older browsers:
#1687864: Bring theme_breadcrumb() up to WCAG 2.0 AA
#1168246: Freedom For Fieldsets! Long Live The DETAILS.

klonos’s picture

I use http://drupal.org/project/modernizr in all my sites anyways. +1 from me.

...there's #1288248: [Meta] Develop a Modernizr API for other Drupal modules btw. And two of its subtasks recently got implemented: #1278504: Call Modernizr.load() from theme .info files (love this one) and #1661746: Create Modernizr.load() API.

rupl’s picture

If people really want this we need some help smoothing out all the 7.x-3.x issues. People report bugs, but one of the pieces that makes it an actual "feature testing solution" is the brand new API that Drupal would surely require if it were in core. It's the piece that allows people to create new versions of the library, and it's about 70% finished at the moment. We need people to use, test, and improve it before making any motion in core.

The other blocker for Modernizr-in-core is keeping the library up to date. If we cannot get some assurance that we will be able to keep the Modernizr library current for minor releases of Drupal 8, then it is a really bad idea to include in core. Period.

We absolutely cannot have the "old jQuery" problem leaking over to Modernizr. In fact, the upstream library will eventually roll over to 3.0 and we would still need to keep it current, even if it's in the middle of D8 release cycle. I've been told that this is an unrealistic request, and it is the sole reason I stopped asking for Modernizr in core a year ago.

If we can avoid the stale upstream lib issue, then let's finish the API and start pulling it in.

catch’s picture

I 100% agree with #37, we can't add front libraries to core then in 5-6 years still be using the same version any more. So either we need to commit to upgrading them regularly, or not do it in the first place.

nod_’s picture

I'm all for not adding it but it means fancy HTML5 features will not work on IE8/9 date input, color input and this kind of things since we can't and won't test for it.

This is a decision to be taking. If it's not acceptable, then we need to have this in core and update it all the time.

We can make sure contrib can add it easily enough though.

rupl’s picture

Well if @catch is for it then that sounds like a reasonable blessing!

In case anyone has compatibility or upgrade concerns: the outward-facing Modernizr API never changes. Even if the underlying feature detections change, you still access the same Modernizr.whatever property from version to version. In the past the main motivation for freezing frontend libraries was that we didn't want to break an API that core depended on. Not an issue with Modernizr upgrades.

Note: there are minor exceptions like Modernizr.flexbox and Modernizr.flexboxlegacy, but that was the result of a change in the web standard which caused huge regressions in every CSS file using flexbox before the update. The issues of that scale are not specific to Drupal and must be dealt with alongside the rest of the web development community.

rupl’s picture

Issue summary: View changes

fixed formatting issue

jessebeach’s picture

I'd like to add my support for Modernizr support in core. I keep running into walls with development that this library would easily solve. We'll see if my need for this library becomes hot enough to force me to write a patch before anyone else, although I don't think I'm right person to do it. Anyone else want to volunteer?

JohnAlbin’s picture

We mentioned feature detection (yet again) in another mobile issue. I think its becoming clear that we need a flexible feature detection model in core.

In the mobile initiative meeting, Jesse talked about using our library system to hold individual tests (or small bundles of logically-grouped tests). Then if a js script requires a specific test, that test library can be loaded. I actually think that's a fairly straight-forward implementation. We could have a glue library on top of that, though I'm not sure its needed since the Library API is pretty good.

Modernizr is already split up into modular feature detection scripts. See https://github.com/Modernizr/Modernizr/tree/master/feature-detects So, that's a big Modernizr++ from me.

effulgentsia’s picture

I 100% agree with #37, we can't add front libraries to core then in 5-6 years still be using the same version any more. So either we need to commit to upgrading them regularly, or not do it in the first place.

FYI: #1787222: [meta] Strategy for updating vendor JS libraries within a major stable version

rupl’s picture

Can we please just set a 302 redirect on this issue to point to my queue? :D

We thought of this months ago and not a single soul has commented on the idea: #1696280: Support individual feature tests within the module. If you scan that issue and follow links you'll see that it's actually kind of gross to extract the feature detects and construct a functional modernizr.js. I have spoken with — nay, begged — Alex Sexton to provide a modular builder on numerous occasions and every time he tells me to wait until they make Modernizr truly modular, which will happen when the upstream library hits 3.0.

It could be wise to try testing some of these concepts on D7 sites before getting crazy with core. There's a place to try these ideas. That place is the Modernizr module. However, I am one dude with a full-time job and ten other drupal-related obligations!!

I would commit the crap out of any patch that lands there to prototype these ideas. If it's such a dire need let's start implementing some code instead of stalling on hand-wavey "core needs this magical, untested silver bullet" type of discussion.

rupl’s picture

Oh, there's also grunt-modernizr which can build Modernizr from the command line. Great for a drush command depending on another JS tool, but Drupal doing that is another thing entirely.

seutje’s picture

I don't think we can wait until 3.0 hits the shelves. Would it be feasible/make sense to port (part of) Modulizr to PHP?

rupl’s picture

It's not impossible, but it will be quite an undertaking. Modulizr (which lives in the customizr branch on Github) currently outputs Modernizr 2.0.6 builds because they don't merge master into it anymore. It can't happen cleanly, and Alex flatly said he's never going to upgrade it until Modernizr 3.0. Result being that Modulizr/customizr is many releases behind.

Here is a more detailed report of how I got Modulizr running: #1030822-8: Modernizr 2 custom builder (client-side for admin UI). I still have the drush command checked in to 7.x-3.x, so we have a Drupal prototype ready to go!

I haven't taken a hard look at grunt-modernizr to see what is being done differently (or if it outputs the latest Modernizr builds). This might be a more attractive option.

seutje’s picture

hmz, the one in the site repo seems more up-to-date (at least dependency-wise)
I just wanna have something without a dependency on node.js, so we can build on top of it

rupl’s picture

Agreed, we can't have a node.js dependency inside Drupal. As John suggested, perhaps we could help finish Modernizr 3.0 and work with a stable release?

'Move all "core" detects outside of modernizr.js' - https://github.com/Modernizr/Modernizr/issues/486

rupl’s picture

Just a note that this issue is not specific to us. There is a Ruby-based project which aims to do an identical task as we've been discussing, whose project page says "wait for Modernizr 3.0"

I posted a comment expressing interest in helping and asking for guidance. Shall we move the party over there for now?

rupl’s picture

Sup peeps, good news! @seutje and I had a chat with Paul about shipping Modernizr 3. He says there have been underlying improvements since the last time this came up and that the majority of it is doable. Hooray! So I propose we attempt to upgrade Modernizr and help out the entire web community at the same time.

There's a summary of our chat on Github, but to keep Drupal peeps posted, here are the notes:

Modernizr 3.0

  • Moving core feature detects out of modernizr.js -- It may not be too hard to do this.
  • Every helper method inside the JS file is now accounted for in the plugin API
  • Porting things out into their own JS files is not too hard
  • Alex brought up dependencies - could be hard.

Todos

  • dependency approach
    • 15 or 20 tests - some depend on other tests
    • webgl extensions test depends on webgl
    • canvas-text depends on canvas
    • We haven't defined semantics for dependencies, right now we manage in some JSON in the builder code -- not ideal.
    • Ideally we get a better way to define dependencies -- may not be a blocker
  • 2 or 3 hour sprint could pave the way for most of this work.

Next steps

  • Make decision about dependencies (AMD, slex??)
  • Do the port
  • Versioning: basic idea is they need a bit more automation for making a new tag and new version every time a change to .js is committed to master. Right now they tag releases, they'd rather just ship master constantly, relying on Travis passing the build.

Converting the 15-20 tests

So... anyone up for some sprinting on Modernizr? I've already committed, but I'll gladly take some help :D

jessebeach’s picture

Issue tags: -mobile, -html5

I'll put in a few hours. Would have to be remote, but I'll get them carved out.

rupl’s picture

@jessebeach just a heads up @seutje and I knocked this out over the last couple days. Thanks for the offer I hope you don't mind :D

The Modernizr team has some other todos involving the packaging of tests, but we're making really great progress at the moment.

jessebeach’s picture

@rupl, I'll never begrudge someone the initiative to get something coded! Just let me know if you'd like me to give another opinion through review. Looking forward to this!

nod_’s picture

It's on https://github.com/Modernizr/Modernizr/issues/713#issuecomment-10171075 :) tests are all AMD on a dev branch. Should be ready soon enough, help needed for testing.

rupl’s picture

Status: Needs work » Needs review
Issue tags: +JavaScript
FileSize
5.67 KB

Ok so the upstream changes to Modernizr 3.x did not bring us to the place we wanted to be for easy integration with Drupal. Time is running short on our side so let's get an MVP patch in before feature freeze.

The patch in this comment adds a small Modernizr build containing only the tests required in other issues that myself and nod_ are aware of. It gets added to the page at the same time as html5.js. End-users who want to take advantage of Modernizr in their projects will certainly want to replace this minimum build with something else, so I'm going to open up an 8.x branch in contrib and hopefully get it to a stable state to address that side of things.

The patch includes a minified Modernizr which gets printed before almost all other JS. This is how it should be, but I wanted to point it out in case we need something to bikeshed on :) Here are the components included in the build:

Tests

  • inputtypes
  • touch
  • elem_details

Note: the inputtypes test does not inject cssclasses into <html>, but the test is accessible via JavaScript by invoking Modernizr.inputtypes.[TYPE]

Extras

  • cssclasses
  • addtest
  • prefixed
  • teststyles
  • testprops
  • testallprops
  • prefixes
  • domprefixes

These components facilitate testing and extending Modernizr, and are all required in order to support the details test since it is a "contrib" Modernizr test.

sun’s picture

Thanks!

This looks RTBC to me. Though, any chance to add the un-minified script(s) until final release? Ideally, we'd simply have both at hand.

I also didn't fully understand your part of the comment about "and [these components] are all required in order to support the details test" — that looks like a huge list and somehow I doubt that all of them are required just for details only :P ;)

Speaking of, we probably want to amend this library with a definition somewhere of what components are contained... Does Modernizr support any kind of notion of build.json or package.json or component.json to easily the exact same currently packaged components, but with their latest upstream code, by coincidence?

rupl’s picture

Happy to explain:

Extras

The extras are helper functions that allow a single Modernizr test to be succinct by abstracting browser differences away from the test itself. For instance, testing battery-level requires that the script loops through the usual suspects in terms of browser prefixes, looking for navigator.mozBattery, navigator.webkitBattery, etc.

These chunks of code are required to use the feature tests as originally written, and the builder at http://modernizr.com/download/ has dependency logic written in to auto-select components that are needed when you select your feature tests.

somehow I doubt that all of them are required just for details only :P

...and as it turns out, code review is a healthy thing. I must have clicked something incorrectly before. While writing this comment I went to go and re-generate the custom build, and not all of them are necessary. This is the list of extras in included in patch #58:

  • cssclasses
  • addtest
  • teststyles
  • prefixes

(un)minifying

Unminified library? Check. At any time, you can follow the link within the source to re-generate the Modernizr build. Minifying is optional on the builder, so we will always have a way to recreate unminified versions. The patch contained in this comment adds the unminified source to the new build so that devs can check it out more easily.

It probably goes without saying but we should always ship the minified library.

Packaging

Does Modernizr support any kind of notion of build.json or package.json or component.json to easily the exact same currently packaged components, but with their latest upstream code

Not without introducing a node.js dependency.. A grunt tool already exists to build Modernizr from CLI. I've been messing with it to offer drush support within contrib for Modernizr. If you follow the link I believe you will see the type of manifest you're thinking of.

It should be possible when Modernizr 3.0 ships. One of the huge wins for 3.0 is that all tests are broken out in their own files, so the library itself can become much more modular. That was the main reason we spent effort trying to get Modernizr 3.0 out the door before attempting this patch. Perhaps by final freeze we can make it happen.

rupl’s picture

It should be possible when Modernizr 3.0 ships.

Just to confirm, yes, you can build from a manifest in the current HEAD: https://github.com/Modernizr/Modernizr/issues/713#issuecomment-10273530

LewisNyman’s picture

I have a request if it doesn't add too much weight, can we add csstransitions and/or cssanimations?

I've used it commonly in pretty much all of my recent client projects and I'm thinking about #1605960: Replace JS animations in core components with CSS transitions

nod_’s picture

By the end we'll have the full test coverage available, it'll just be a matter of adding a library to get your tests (hopefully) we're just going small and simple for the initial patch. We have shit to figure out with the Modernizr guys :)

rupl’s picture

@sun, re-reading the comments today I realized the current patch includes a half-solution to your question about managing versions of the upstream library. Here's the URL from patch in #58

http://modernizr.com/download/#-inputtypes-touch-cssclasses-addtest-teststyles-prefixes-elem_details

Every Modernizr build (minified or not) includes a URL that takes you back to the http://modernizr.com/download/ page where you can re-generate that same library using the latest source code. So you can always go back and reliably rebuild the library (for minor upgrades between point releases of Drupal and so forth). It's not 100% automated, but for the purposes of quickly generating a core patch it will work.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! The original/minified part was my biggest concern and you resolved it. :)

(Wow, dare I say that code/coding-style looks pretty poor? ;))

Modernizr is definitely the de-facto standard solution today that is backed by a large community, so Drupal should absolutely adopt it.

jessebeach’s picture

I tested as well. Looks great. Looking forward to having this in core and to pluggable tests.

Fantastic job rupl and nod_ for driving this. Thank you so much!

webchick’s picture

Assigned: Unassigned » Dries

Typically Dries needs to sign off on these sorts of patches, but plus a billion from me.

webchick’s picture

Category: task » feature
Priority: Normal » Major

Also fixing metadata.

MustangGB’s picture

I may have missed a post on this but if HTML5shiv is already in core and Modernizr also contains HTML5shiv then what's the point in having to maintain dependency on both libraries?

Shouldn't this patch deal with ripping out HTML5shiv as well, or is there another issue for this that is postponed on this issue being fixed?

JohnAlbin’s picture

I may have missed a post on this but if HTML5shiv is already in core and Modernizr also contains HTML5shiv then what's the point in having to maintain dependency on both libraries?

That's a good point! I'm looking into that right now. But it should not hold up this patch. (Which is plus a billion and 1 from me.) I'll investigate, report back shortly and open a follow-up issue if needed.

JohnAlbin’s picture

@akamustang: The patch in #58 contains a minimal build of Modernizr which does not include the HTML5 shiv. You can see what components it includes by following the URL in the modernizr.min.js file. http://modernizr.com/download/#-inputtypes-touch-cssclasses-addtest-test...

After this minimal build of Modernizr is in core we can look at the HTML5 shiv / Modernizr integration in more detail.

MustangGB’s picture

Thanks for the link. I did notice we were using a custom (very small) build so semi-guessed it wasn't included. Guess this can be a separate clean-up issue.

rupl’s picture

Comment #69 is correct; this Modernizr build does not contain the html5shiv.

It would be possible to combine them (see comment #2 of this issue which lists Modernizr as a way to include the shiv), but right now we simply want this patch committed as is. This will get Modernizr into core before feature freeze. I thought about writing the patch so that it takes the shiv out, but people would probably pour in here to debate it and we don't have time at the moment.

Bottom line, removing the standalone shiv and including it within Modernizr is an optimization that can happen after Dec 1st.

sun’s picture

I totally agree with #71.

webchick’s picture

Title: Add a feature detection solution » Change notice: Add Modernizr to core
Assigned: Dries » Unassigned
Category: feature » task
Priority: Major » Critical
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Spoke about this with Dries (he had to run, so paraphrasing). Based on the description here, this library looks good to go. It's nice that we managed to get a build so small, to cut down on front-end performance concerns. This also necessitated tackling #1787222: [meta] Strategy for updating vendor JS libraries within a major stable version since of all the libraries we're adding to D8 core this time around, this is one that's almost guaranteed to evolve several hundred times during D8's lifetime, the way the web is going.

Also, in looking more at what Modernizr provides, I think I mis-categorized it as a feature; this is pretty essential to well-functioning HTML5 stuff in older browsers. Therefore....

Committed and pushed to 8.x. :) Thanks!

Let's get a change notice for this, especially for the parts of Modernizr that are/are not included because it sounds like there is some confusion about that.

rupl’s picture

Status: Active » Needs review

Change notice has been drafted: http://drupal.org/node/1852968

effulgentsia’s picture

Title: Change notice: Add Modernizr to core » Add Modernizr to core
Priority: Critical » Major
Status: Needs review » Fixed

Thanks. I think the change notice is fine as-is, but if someone feels inspired to add a code example for "a module can now provide touch-specific styles within its admin interface", that would make it even better. Also, is there an example of any D7 code that this was able to deprecate (i.e., "here's the lame way you used to do this in Drupal, and here's the shiny new way")?

LewisNyman’s picture

I've added a pretty minor example to the change notice just because I've never been involved in writing a change notice before and it looks like fun!
http://drupal.org/node/1852968

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

sun’s picture

Issue tags: +polyfill

Retroactively tagging polyfill related issues.

ry5n’s picture

I’m starting work on integrating the Seven Style Guide into core, and already I need a Modernizr CSS test for boxshadow, which I see isn’t in the current minimal build. What’s the strategy for core going forward?

jessebeach’s picture

What's the purpose of testing for box-shadow? The effect of not apply a box shadow, either by using a class to scope it to UAs that support box shadow, or by declaring box shadow in the CSS and having it ignored by a UA that doesn't support it, is the same -- no box shadow.

ry5n’s picture

Normally, yes, I’d just let older UAs not have the shadow. But this situation is to apply a custom focus ring, which includes outline: none. I want UAs that don’t support box-shadow to retain their default focus ring:

.boxshadow .button:focus {
  outline: none;
  border-color: #40b6ff;
  -webkit-box-shadow: 0 0 0.5em 0.1em hsla(203, 100%, 60%, 0.7);
  -moz-box-shadow:    0 0 0.5em 0.1em hsla(203, 100%, 60%, 0.7);
  box-shadow:         0 0 0.5em 0.1em hsla(203, 100%, 60%, 0.7);
}
jessebeach’s picture

Ok, that makes sense. Seven should be able to add a test to the Modernizr object in its JavaScript.

gkatsanos’s picture

And this is why some of us weren't in favor of adding libraries and tools like that into core. Its jQuery 1.4.x all over again.

jessebeach’s picture

It's not quite the same situation. Modernizr is modular. We can add tests to it. What we committed was the basic framework and tests for touch and details.

In ry5n's case, he wants to add a test for box-shadow, but we don't want this test added in core because really, only the Seven theme needs it. So the Seven theme can add it.

The patch I added is a monkey-patch on the Modernizr library in core. It adds the testProp method that we seem to be missing. I just lifted it from here: http://modernizr.com/download/#-boxshadow-cssclasses-testprop-testallpro...

The code isn't what we'd want to commit, but it shows that we just need an incremental improvement to what we have, rather than tossing it out.

rupl’s picture

@gkatsanos please read comment #38 in this issue where D8 maintainers pledged to integrate upstream changes to Modernizr, which is the exact opposite of your claim. Also adding another test during dev cycle is a totally legit scenario. Once core ships you'll be able to use the contrib module to add all the tests you want.

I'm moving the additional test into a new issue because this one is closed, contains a change notice, and basically is dead now.

#1987346: Add boxshadow to Modernizr

rupl’s picture

@jessebeach that's an interesting approach. You'll need testAllProps in addition to testProp.

I still think we should let this issue rest in peace.

ry5n’s picture

@jessebeach @rupl Thanks. Happy to let this issue rest.

xjm’s picture

Issue tags: -Needs change record

Untagging. Please remove the "Needs change notification" tag when the change notice task is complete.

xjm’s picture