Problem/Motivation

Using the "Link image to" feature of the picture formatter causes a fatal error:

Fatal error: Cannot use object of type Drupal\Core\Url as array in /core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php on line 129

Steps to reproduce:

  1. Set up a responsive image mapping
  2. Create a content type with an image field
  3. Set the default display mode to use the responsive image formatter and set the "Link image to" to "Content"
  4. Create a node
  5. View the node
  6. You see a WSOD with the error mentioned above

Proposed resolution

tbd

Remaining tasks

tbd

User interface changes

tbd

API changes

tbd

Original report by [username]

tbd

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

RainbowArray’s picture

Priority: Normal » Major
Issue tags: +TCDrupal 2014

Using an image as a link is fairly common, so if that is causing a WSOD when Responsive Image module is being used, that's a pretty big problem. Bumping this to major. Will test to see if we can reproduce, and if so, start working on finding a solution.

chrischinchilla’s picture

Seeing if I can reproduce…

chrischinchilla’s picture

Yup, did a fresh D8 install, followed steps above and get WSOD with -

10/08/2014 12:22:01.291 httpd[54712]: PHP Fatal error: Cannot use object of type Drupal\Core\Url as array in /Library/WebServer/Documents/drupal8/core/modules/responsive_image/src/Plugin/Field/FieldFormatter/ResponsiveImageFormatter.php on line 129

chrischinchilla’s picture

Issue tags: +Needs tests
chrischinchilla’s picture

Will make some initial investigations to see what the issue is, might be beyond me, but you never know!

Here's a patch…

I'm not sure if I took the correct path, but I looked into 'ImageFormatter.php' and there was also viewElements function with very similar code. I replaced the erroring code with the code from there and things work.

After applying, make sure you clear cache and the responsive image with a link now works.

chrischinchilla’s picture

chrischinchilla’s picture

FileSize
1.44 KB
chrischinchilla’s picture

Don't know what was up with that other patch, try this one!

RainbowArray’s picture

Status: Active » Needs review

Looks good. Let's see what testbot says! Go testbot go!

chrischinchilla’s picture

As discussed with mdrummond he mentioned that a test would have spotted there had been a problem in the first place.

I have attached an attempt at a test, but I don't think it's completely correct and maybe doesn't really test anything.

Wasn't sure how to test multiple instances of a field or multiple fields and in fact, looking through other tests I don't think there are tests for fields linking to a content item anywhere, which may or may not be a problem. Or maybe this test isn't even needed, but here it is.

Status: Needs review » Needs work

The last submitted patch, 10: resp_image_link-2315077-9043095.patch, failed testing.

RainbowArray’s picture

Looks we have two issues tackling the same thing. Need to determine which has the best approach. This one has a start on tests, which is good.

RainbowArray’s picture

Version: 8.1.x-dev » 8.0.x-dev
attiks’s picture

swentel’s picture

Status: Needs work » Closed (duplicate)

Let's mark this as duplicate in favor of #2254071: [PP-1] Responsive images can't be linked to Content - note, this is just done on the basis that the other one is older.