Enable responsive_image.module by default for standard install profile

This is probably obvious, but picture shouldn't be enabled for all images, just those that would benefit from major breakpoint source choice. People using picturefill and other responsive image libraries in the wild have mentioned that switching sources between a 100x100px 2kb image and a 60x60px 1.5kb image is more overhead than it's worth, so user pictures, logos, etc, are probably not good candidates for the new formatter.

Because image sizes are so site specific would it be worth writing a function to run on the manage display page that checks if breakpoint module is enabled, and if a field is using an image formatter with an image style that outputs dimensions greater than some arbitrary value (maybe 500) sets a message suggesting the picture formatter?

Just throwing this out there, but should this entire module be rolled into Image module? It's pretty confusing atm to have both a Picture and an Image module, and it seems like this module expands on the functionality already available in image module.

Just a thought. We can always enable picture as part of standard profile first, before answering that question, but that's another way of solving it since Image module is already turned on by default.

Since it's been a year since this was last discussed, and since this is tagged as an issue with the breakpoints module, do we want to revisit turning this on? FYI, there's another issue to rename Picture to the Responsive Images module, which may clear up some of the confusion: #2124377: Rename "Picture" module to "Responsive Image" module.

...clarification ;)

picture.module already renamed to responsive_image.module.

#2513604: Create default responsive image styles adds default responsive image styles. I also checked Bartik, and it already has CSS for responsive images.

So if we get the default styles in, I think it's worth considering turning on the Responsive Image module in the standard profile.

Since default responsive image styles are now in core, I think it's worthwhile thinking about turning this on by default now.

I think this belongs in 8.1, repeating what I said in #2061377-75: [drupalImage] Optionally apply image style to images uploaded in CKEditor 5:

I talked to @webchick and @mdrummond about this.

Drupal 8 claims to do All The Right Responsive Things, but without this issue, no inline images are ever responsive. Which is kinda sad.

OTOH, #1855412: Enable responsive_image.module by default for standard install profile is also not done yet, so actually, out of the box, Drupal 8 doesn't do any responsive images things at all.

Given that RC1 is in exactly one week, I don't think it's realistic to get all this done by then. Especially because this would benefit from real-world feedback. I think it'd be better to put this patch in a D8 contrib module during 8.0.0, gather real-world feedback, and move it into core in 8.1.0. Then 8.1.0 can say "responsive images support has now matured, it is now enabled by default, and includes support for inline images also".

It's very unfortunate, but from a release manager POV, I think that's the most responsible thing to do.

Well, we support all of Drupal 8. This issue is about the Article content type swapping out some supported code for other supported code. The fact that something is in standard profile increases release management risk only slightly, and the benefit is more than slight IMO.

Ok, happy to be wrong!

Fixed test.

+++ b/core/modules/rdf/src/Tests/StandardProfileTest.php
@@ -229,7 +229,7 @@ protected function doFrontPageRdfaTests() {
-    $image_uri = ImageStyle::load('medium')->buildUrl($image_file->getFileUri());
+    $image_uri = ImageStyle::load('max_325x325')->buildUrl($image_file->getFileUri());
     $expected_value = array(
       'type' => 'uri',
       'value' => $image_uri,
@@ -260,9 +260,11 @@ protected function doArticleRdfaTests() {

@@ -260,9 +260,11 @@ protected function doArticleRdfaTests() {
 
     // @todo Once the image points to the original instead of the processed
     //   image, move this to testArticleProperties().
+    $image_file = $this->article->get('field_image')->entity;
+    $image_uri = ImageStyle::load('max_325x325')->buildUrl($image_file->getFileUri());
     $expected_value = array(
       'type' => 'uri',
-      'value' => $this->imageUri,
+      'value' => $image_uri,

These are the only changes I do not 100% understand.

Thanks! That makes sense.

Now doing manual testing.

Works as expected:

• fresh install: responsive images for Articles
• updated install: no responsive images for Articles

I don't think there's anything left to be done here.

Wrt to the "rc target" tag see https://groups.drupal.org/node/424518. Assigning this to @webchick as it is a product decision.

...

What changed since our IRC discussion the other day?

#40 @moshe in #13:

Well, we support all of Drupal 8. This issue is about the Article content type swapping out some supported code for other supported code. The fact that something is in standard profile increases release management risk only slightly, and the benefit is more than slight IMO.

Spoke about this with @xjm, @Wim Leers, @effulgentsia and @Dries today.

We're all definitely on board with getting this in front of as many users as possible, as quickly as possible. The front-end performance benefits are quite large (and as @attiks notes, even some of the most D8-knowledgeable people are getting this wrong right now), as well as the huge shot in the arm for the mobile capabilities of D8.

However, the concern with this request is:

a) it comes very late in the game (we have about 4 days left, including a weekend, to review/commit all outstanding patches that are going to make it into 8.0.0)
b) unlike some of the other patches in that list, this change could be made in 8.1.x (and be an awesome functionality/performance enhancement) and not forced to be deferred until D9 if it isn't done
c) we really, really, really, really, really, really, really, really, really, really, really do not want to introduce any patches at this point that could destabilize the release in any way. In a funny catch-22; because this module isn't in front of every user by default, that means there's been very little testing of it outside the maintainers/committers. So suddenly putting it in front of users has the risk of creating more urgent problems to solve and then be limited in our ability to solve them because it's a default option now.
d) there are still some big outstanding issues such as #2061377: [drupalImage] Optionally apply image style to images uploaded in CKEditor 5 (which is making great progress, yay) that would make me nervous to enable this by default, given the module doesn't yet work to user expectations (which is that if you say D8 supports responsive images out of the box, it works consistently for all images, regardless of their origin).
e) I also have a further concern that the site builder UI is not a site builder UI; it still requires in-depth knowledge of various HTML5 details in order to even understand what to do. It's worth pointing out that this concern, however, wasn't shared with Dries, who pointed out we have lots of sub-optimal site builder UIs and people still manage. ;)

Based on all of these factors, however, we felt a lot more confident pushing this to 8.1.x than trying to get it in in the next 4 days. It's still accurate to say that Drupal 8 ships with responsive image support out of the box (just as it's accurate to say that Drupal 8 ships with multilingual support out of the box); the fact that you have to configure your content types to use it is a fact of life regardless if it's enabled by default in Standard or not right now.

That said, I'd love to work with you guys sometime during RC/post-release to come up with a definitive "hit-list" of issues to resolve before we can do this, and aim to commit this patch early in the 8.1.x cycle, as it's undisputedly a huge win.

For what it's worth, I'm completely on board with this plan to do this for 8.1 (assuming we finish the hit list).

The challenge for e) is that we don't really want to introduce the legacy of another site building tool that is architected after complex hard to understand models. Its a reflection of in my eyes the poor state that this is in with browsers and the spec. I hope we can find an ingenious way around it.

This is just a reroll and an issue reactivation :)

I think we're still missing the hit-list from #43. Bumping back to CNR and tagging for webchick.

The big one if we want to do this is #2061377: [drupalImage] Optionally apply image style to images uploaded in CKEditor 5. I'd still love to get that into 8.1, but I could definitely use some help.

@catch For review to happen, I think we expect a lot more community feedback on what the hit list should be.

Really sorry, this one definitely fell off my radar. :\

My suggestion for a next step would be to come join one of the semi-weekly UX meetings and provide a walkthrough of the current functionality so a bunch of UX-minded individuals can help ascertain a must-have/should-have/could-have list.

It makes sense to enable and use *the functionality* of responsive images in the default install profile, but this UI, it's not very great :)

Do people know about other authoring tools or apps that provide a user interface for configuring these things? Would be good to get some inspiration if possible.