Move picture into core

...
• Polyfill works in modern browsers (IE9+)
• IE8 isn't working, because is doesn't understand media queries

...

I realize there's talk about dropping IE8 support in D8, but wouldn't this work in IE8 too if a polyfill (like matchmedia.js for example) was used?

In #1465948: [meta] Drop some IE8 coddling from Drupal core and #1645494: Allow core themes to display single-column on IE8: do not compensate for lack of media query support, we decided that it is not core's responsibility to compensate for IE8's lack of media query support: that IE8 falling back to mobile layouts (and by extension, image variants) is ok. So I'd like to restate #1 as: if a contrib module like Respond.js were enabled, would the picture polyfill work on IE8, or would something else be needed, and if so, what?

Hey, just figured I’d chime in. I know supporting IE8 is a much larger discussion, but as far as Picturefill goes: Scott Jehl, Paul Irish, and Nicholas Zakas whipped-up an awesome matchMedia polyfill that we’ve thoroughly tested with Picturefill.

https://github.com/scottjehl/matchMedia.js

As I understand the Picture module facilitates mapping of image caches to different groups of breakpoints.

Steps followed to configure Picture module and test the same:

1. Created article with image
2. Enable Breakpoint module
3. Enabled Picture module
4. Visited Configuration page Picture mapping admin/config/media/picturemapping
5. Added a label 'My Article' to Group Bartik
6. Mapped mobile to Thumbnail, narrow to Medium and wide to Large
7. Visited Content Types settings page at admin/structure/types/manage/article/display
8. Changed Article Image format from Image to Picture
9. Clicked on the settings icon
10. Mapped to Picture mapping 'My Article' created earlier & set Fall-back image to Automatic. Don't forget to click save here!
11. Applied patch at: #1815602-43: Introduce a polyfill for matchMedia
12. Visited article with image and resized browser, the breakpoints and image sizes mapped using Picture configuration did not apply. The full sized image was loaded and the image resized as the browser was resized. The testing was carried out in Chrome

The Configured breakpoints did not seem to apply! Is there any configuration that I have missed?

Output for the picture:

<div class="field-items">
<div class="field-item even" rel="og:image rdfs:seeAlso" resource="http://192.168.99.131/drupal-8/sites/default/files/field/image/Blue%20hills.jpg">
<img typeof="foaf:Image" src="http://192.168.99.131/drupal-8/sites/default/files/field/image/Blue%20hills.jpg" width="800" height="600" alt="">
</div>
</div>

Also attached screen shot of configuration & updated steps at #25

Looks like I did not click on save. It's working fine now.
Shouldn't there must be an alert to the user requesting him to click on save? I only used the update button!
Works perfectly on all breakpoints.

Works perfectly on following the steps at #25.

I believe we need #1815602: Introduce a polyfill for matchMedia in first, so marking this postponed.

#36: picture-36.patch queued for re-testing.

Tested this and it works really nicely. This is going to be fantastic.

1. On admin/config/media/picturemapping/add, we need better form element descriptions.
2. Typo in s/on/one: "For each breakpoint you can select a corresponding image style, you have to select at least on image style.". Also this seems like duplicate text.

+++ b/core/modules/picture/picture.module
@@ -0,0 +1,328 @@
+/**
+ * Picture uri callback.
+ */

Duplicate Doxygen

If you reroll for #48
+ // Create user. + $this->admin_user = $this->drupalCreateUser(array('administer pictures', 'access content', 'access administration pages', 'administer site configuration', 'administer content types', 'administer nodes', 'create article content', 'edit any article content', 'delete any article content', 'administer image styles')); + $this->drupalLogin($this->admin_user);
Can these go one per line as per coding standards
Oh and this is awesome btw!

Thanks

+++ b/core/modules/picture/picture.moduleundefined
@@ -0,0 +1,329 @@
+    'website' => 'https://github.com/attiks/picturefill-proposal',

Like we did with mediaMatcher, let's point this at a Drupal URL rather than a github URL for one specific individual (especially one with "proposal" in the name :D)

+++ b/core/modules/picture/picture.moduleundefined
@@ -0,0 +1,329 @@
+function theme_picture_formatter($variables) {
...
+function theme_picture($variables) {
...
+function theme_picture_source($variables) {
...
+function picture_get_image_dimensions($variables) {

We need to make sure these functions all have PHPDoc above them, as well as @param / @return docs. Part of the docs gate.

Apart from this, great work! happy to commit it. :)

Committed and pushed to 8.x. YAY! :D

Couple remarks on the code that got committed :

+++ b/core/modules/picture/lib/Drupal/picture/Plugin/Core/Entity/PictureMapping.php
@@ -0,0 +1,161 @@
+/**
+ * Defines the Picture entity.
+ * ¶
+ * @Plugin(

Whitespace slipped in.

+++ b/core/modules/picture/lib/Drupal/picture/Plugin/field/formatter/PictureFormatter.php
@@ -0,0 +1,189 @@
+          if (isset($picture_mapping->breakpointGroup->breakpoints[$breakpoint_name])) {
+          $breakpoint = $picture_mapping->breakpointGroup->breakpoints[$breakpoint_name];

indent issue - and not clear where that if block ends

+++ b/core/modules/picture/lib/Drupal/picture/Plugin/field/formatter/PictureFormatter.php
@@ -0,0 +1,189 @@
+              // Make sure the multiplier still exists.
+              //if (!empty(array_intersect($multiplier, $breakpoint->multipliers))) {

leftover commented line ?

+++ b/core/modules/picture/picture.info
@@ -0,0 +1,9 @@
+dependencies[] = config

Why should this have a dependency on config.module ? config.module is only about config UI, not about the config system

+++ b/core/modules/picture/picture.module
@@ -0,0 +1,379 @@
+    // add source tags to the output.

Sentence should be capitalized

+++ b/core/modules/picture/picture.module
@@ -0,0 +1,379 @@
+    // output the fallback image.

Ditto.

+++ b/core/modules/picture/picturefill/picturefill.js
@@ -0,0 +1,126 @@
+        // if there's no media specified, OR w.matchMedia is supported

Ditto, plus missing trailing point - plus, that's not a real sentence :-)

Also, can the picture.info description be expanded past "Picture element"? That doesn't tell me much.

Needs a follow-up for coding standard issues.

+/**
+ * @file
+ * Definition of Drupal\picture\PictureFormController.
+ */

All of these should be like:

Contains Drupal\picture\PictureFormController.

+  /**
+   * Load breakpoint group.
+   */
+  protected function loadBreakpointGroup() {

+  /**
+   * Test picture administration functionality.
+   */
+  public function testPictureAdmin() {

More of these around that should be third person.

Cross post. Restoring status.

I don't know if the bot can read that patch, but I can't. :)

looks good, thanks !
(checked with @alexpott that picture and breakpoint indeed have no reason to depend on config.module)

green

Note: The interdiff is backwards :)

But the current patch looks good.

Committed and pushed to 8.x.

Do we have issues open for actually using this by default? I would think we want to ship with picture enabled in standard install and picture formatter on the image fields in Article content type and on User entity (i.e. user picture).