Reusing a field also copies display settings for non-existing displays and displays not enabled on the source

Pushed fixes and some test to the MR.

I think there was a misunderstanding in how getForm/ViewDisplay work, as they auto-create non-existing displays. It checked the status there, but they are enabled by default.

I did keep the status check and switched to load with a check that the entity exists before the status call. I can see that updating a disabled *target* view display could be useful if you ever enable it again, it won't hurt now and you can review it when you do so and I guess that status() really should have been an isNew(). But didn't make that change yet.

Test coverage could be extended to cover that case explicitly as well.

Fixed coding standards I think. FWIW, I think phpcs is wrong there, it's an array in a chained method call and now the save() calls are on the wrong line. But whatever :)

@Berdir the Steps to Reproduce are bit confusing.
In the 2nd step does it mean disabled again instead of displayed again?

Going by the functional test cases. The MR does seem to fix the reported issue.
The disabled displays are not copied to the target entity on field reuse after applying the changes from the MR.

Steps used for testing

• Created a custom view mode, enabled it for Article node type.
• Created a one more view mode. enabled it for Article node type.
• Added a new field on Article content type, configured it to display on both the new display modes.
• Disabled the 2nd display mode from Article node type.
• In Page node type reused the newly added Article field. Observed the manage display for Page node type.
• Before applying the MR changes, it did copy the disabled view display on Page node type.
• It only created the enabled view display after applying MR.

Looks like it will require more test coverage. Looking into the same.

Yes, that should say disabled, clarified a bit.

What test coverage do you think is missing?

I'm wondering if we should special case the default modes and allow creating that in case that it doesn't exist? It looks like it's not guaranteed that all entity types create the default view mode for all bundles. Node module is ensuring that the default modes are created in node_add_body_field.

> I'm wondering if we should special case the default modes and allow creating that in case that it doesn't exist? It looks like it's not guaranteed that all entity types create the default view mode for all bundles.

Maybe, if you create a new bundle and the very first field you add is a reused one then I guess it could not exist yet, I kept the logic that happens when we get no existing display configuration. That also would also fix that test fail I guess.

Without that, I did switch to using load on purpose because creating config entities when we're not going to use them is inefficient, but I guess if we special case default then that makes sense.

So even though I requested push access I couldn't rerun the failing javascript test. So I rebased to get them to run again.

Ran the test-only feature and do see tests correctly failing.

Can the issue summary be updated though, specifically the proposed solution