Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
It's fantastic usability that add a new term comes up with the form for adding still another term, but mass addition of text formats is not something we want to promote; instead going to the listing makes the most sense in the large majority of likely cases (that is, people using the standard installation profile).
That is, submitting admin/config/content/formats/add should take you to admin/config/content/formats (where, among other things, you could change the order in which your new text format will appear with the existing ones).
Comment | File | Size | Author |
---|---|---|---|
#15 | drupal8.filter-format-form-redirect.15.patch | 5.63 KB | agentrickard |
#10 | drupal8.filter-format-form-redirect.10.patch | 1.49 KB | agentrickard |
#7 | drupal8.filter-format-form-redirect.6.patch | 1.64 KB | sun |
#5 | 1053518-filter-form-redirect.patch | 810 bytes | agentrickard |
#1 | 1053518-format-add.patch | 514 bytes | agentrickard |
Comments
Comment #1
agentrickardSimple patch.
Comment #3
sun#1: 1053518-format-add.patch queued for re-testing.
Comment #5
agentrickardI see no point to adding a test here.
Comment #7
sunThis bugs me since D7 and I have no idea why I didn't add a redirect there. Verifying this expectation in tests is a piece of cake though :)
However, we might want to wait for #1779026: Convert Text Formats to Configuration System, since I think the changes here conflict with that patch.
Comment #9
sunI had a chance to double-check the changes here against #1779026: Convert Text Formats to Configuration System tonight, and the two changes do not conflict with each other, so we can happily move on, once the test failures are fixed.
This patch will only conflict with the follow-up clean-up issue #1881664: Clean up and complete text format config entity conversion. Ideally, we get this patch in before work starts to happen over there.
Comment #10
agentrickardRe-reading the o.p., the tests break because the request was only to redirect when adding a new filter.
This patch adjusts that behavior and fixes the failing tests that result.
I believe the assumption is that updating a filter should return you to the filter form, so you can review changes. Adding a filter should take you to administer filters (or, alternately, to view the filter you just created.)
Comment #11
sunmmm, I don't think that makes sense though. The general form submission redirect pattern throughout core is to redirect to two discrete locations, depending on the thing you added/edited:
- for content/viewable entities, we redirect to the view.
- for configuration/unviewable entities, we redirect to the listing page.
We do not make a differentiation between add/edit currently.
The only exception to that rule is the taxonomy term add form, which redirects back to itself, but that's with the concrete idea and purpose that people who are using that form often want to add multiple terms subsequently. It's a relatively bold assumption though, and actually we should remove that special behavior and add a separate "Save and add another" submit button to the term-add form.
For the text format form, there's nothing to review after submitting. In fact, I think the lack of a redirect stems from the D6 user interface of text formats, which had the filter sorting + filter settings as separate tabs/pages and thus, that configuration was not available until a format was saved - hence, no redirect. That's no longer the case though.
Comment #12
agentrickardOK, then we redirect in both instances, and that will mean removing some of the assertFieldByName pieces in the current tests. (Or using the pattern of durpalGet in the later patch).
Comment #13
sunYeah, drupalGet() back to the form. We obviously don't want to remove test coverage. ;)
That said, the
assertUrl()
changes from #7 should be taken over as well, since that's explicit test coverage for verifying that we end up at the expected URL after posting.Comment #14
agentrickardGot it.
Comment #15
agentrickardWith passing tests (locally).
Comment #16
sun#15: drupal8.filter-format-form-redirect.15.patch queued for re-testing.
Comment #17
sunGlad that it still passes. It's about time to fix this. :)
Comment #18
catchMakes sense. Committed/pushed to 8.x.