Certain configuration can end up to fatal error.

Steps to reproduce: Add a Revive Adserver Zone field on an entity bundle. Do not check the "Select invocation method per entity."

When the entity is rendered in a view mode that contains the field, it will result to a fatal error, as there is no invocation method, and also there is no way to select an invocation method.

Comments

efpapado created an issue. See original summary.

efpapado’s picture

StatusFileSize
new5.69 KB

Proposing patch.

efpapado’s picture

Status: Active » Needs review
szeidler’s picture

Hi,

thanks for creating the issue and proposing a patch.

When the entity is rendered in a view mode that contains the field, it will result to a fatal error, as there is no invocation method

I can confirm this.

and also there is no way to select an invocation method.

This is actually not true, but maybe not that super obvious. In that case you need to specify the invocation method on the "Manage display" settings of your entity. Then there will not be a fatal error.

But I agree, that a global setting could make sense here and such a fatal error should not happen in any case. Then there wouldn't be any further configuration required on the per-block or per-entity base.

szeidler’s picture

Attaching a patch, that tests the given scenario. It should fail and will be used to prove that the patch #2 is fixing the issue.

szeidler’s picture

And a patch with proper test and patch #2 included. That should ideally pass successfully.

szeidler’s picture

StatusFileSize
new10.65 KB

New test against the current dev, using the new test trait.

efpapado’s picture

and also there is no way to select an invocation method.

This is actually not true, but maybe not that super obvious. In that case you need to specify the invocation method on the "Manage display" settings of your entity. Then there will not be a fatal error.

You're right, I missed it!

Nice work with the tests :)

szeidler’s picture

Status: Needs review » Needs work

We agreed in a side-disussion, that it should be enough, just to make sure, that the fallback field formatter is the "Async" method, simply because we expect it to be the most used one. That will prevent the fatal error.

We should document this behavior and also fact, that the Invocation method can be changed on the "Manage display" field formatter level.

A global invocation method is nice, but we don't see much use-cases, where someone would choose for example "iframe" globally for the site.

szeidler’s picture

szeidler’s picture

Title: Provide global fallback default invocation method » Use "async" as a revive field formatter fallback, if no invocation method was provided
szeidler’s picture

Status: Needs work » Needs review
StatusFileSize
new8.08 KB
new4.53 KB

Here's a patch for using the "async_javascript" method as a defaultsetting. If the test returns green, the fatal error should not happen anymore.

  • szeidler committed bce43d6 on 8.x-1.x
    Issue #3042324 by szeidler, efpapado: Use "async" as a revive field...
szeidler’s picture

Status: Needs review » Fixed

Merged into the dev-release. Thanks for the contribution!

Status: Fixed » Closed (fixed)

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