Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in text filter format overview submit when disabled formats exist #4419

Closed
indigoxela opened this issue May 24, 2020 · 5 comments · Fixed by backdrop/backdrop#3151

Comments

@indigoxela
Copy link
Member

indigoxela commented May 24, 2020

Description of the bug

When at least one format is disabled and the filter format overview form gets submitted (changing the weight of a format), an additional empty format gets created, causing several php notices and warnings.

Steps To Reproduce

To reproduce the behavior:

  1. Add a format
  2. Disable that format
  3. Go to admin/config/content/formats, (reorder), submit

Actual behavior

    Notice: Undefined index: aaanotherformat in filter_admin_overview_submit() (line 114 of ...core/modules/filter/filter.admin.inc).
    Notice: Trying to get property 'weight' of non-object in filter_admin_overview_submit() (line 114 of ...core/modules/filter/filter.admin.inc).
    Warning: Creating default object from empty value in filter_admin_overview_submit() (line 116 of ...core/modules/filter/filter.admin.inc).
    Notice: Undefined property: stdClass::$name in filter_format_build_format() (line 293 of ...core/modules/filter/filter.module).
    Notice: Undefined property: stdClass::$format in filter_format_save() (line 362 of ...core/modules/filter/filter.module).
    Notice: Undefined property: stdClass::$format in filter_formats() (line 797 of ...core/modules/filter/filter.module).
    Notice: Undefined property: stdClass::$format in filter_formats() (line 808 of ...core/modules/filter/filter.module).
    Notice: Undefined property: stdClass::$format in filter_admin_overview() (line 61 of ...core/modules/filter/filter.admin.inc).
    Notice: Undefined property: stdClass::$format in filter_get_roles_by_format() (line 923 of ...core/modules/filter/filter.module).

You might have guessed: "aaanotherformat" is the name of the disabled one.

Additionally a new file gets created in config directory: filter.format..json
With this content:

{
    "_config_name": "filter.format.",
    "weight": "0",
    "name": "",
    "cache": true,
    "status": 1,
    "editor": null,
    "filters": []
}

Additional information

This popped up while I was trying to figure out a possible solution for another (unrelated) Issue but it's reproducible on a different install.

Both have php 7.2, one is the latest dev (git).

Deleting that additional broken filter config file and enabling the filter via UI and then deleting the newly created broken filter config again "solves" the problem. At least the notices go away and no additional config files get created (as long as there's no disabled format).

@indigoxela indigoxela changed the title Regression in text filter format submit when disabled formats exist Regression in text filter format overview submit when disabled formats exist May 24, 2020
@indigoxela
Copy link
Member Author

indigoxela commented May 24, 2020

Crazy, the fix is a one-liner...

The trick is to get the same filter list (filter_formats() with same parameters) in both, function filter_admin_overview() and function filter_admin_overview_submit(). If that's not the case, disabled ones create empty formats on submission as they exist in $form_state['values'].

To test the PR follow the instructions in the Issue description.

@herbdool
Copy link

Tested and reviewed

@jenlampton
Copy link
Member

Is this a duplicate of #4407?

@indigoxela
Copy link
Member Author

Is this a duplicate of #4407?

No, it's unrelated. There's no solution yet for the other issue.

@quicksketch
Copy link
Member

Thanks! Merged backdrop/backdrop#3151 into 1.x and 1.16.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants