Problem/Motivation

On text format overview page (admin/config/content/formats) configure links are supplied with destination parameter. So there is no point to set redirect in format edit form. Currently submitting edit form takes you the overview page, even when you have removed the destination parameter from URL. This is a bit annoying.

Proposed resolution

Move $form_state->setRedirect('filter.admin_overview'); from FilterFormatFormBase to FilterFormatAddForm.

Remaining tasks

Discuss and create a patch.

User interface changes

No changes, because of destination parameter in configure operation links.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Chi created an issue. See original summary.

Chi’s picture

Title: Edit text format form should not redirect a user to to text format listing » Edit text format form should not redirect a user to text format listing
msankhala’s picture

Status: Active » Needs review
FileSize
1.11 KB

Here is the first version of the patch.

Status: Needs review » Needs work

The last submitted patch, 3: 2977054-3.patch, failed testing. View results

veerasekar.r89’s picture

Moved $form_state->setRedirect('filter.admin_overview'); from FilterFormatFormBase to FilterFormatAddForm.

Status: Needs review » Needs work

The last submitted patch, 5: edit-text-format-form-should-not-redirect-2977054-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

maboresev’s picture

I have applied @veerasekarr89 patch to my drupal and it works properly. I don't know why It doesn't passed the tests.

The proposed solution works properly.

msankhala’s picture

@veerasekar.r89 How the patch #5 is different from the patch in #3. I don't see any difference between these two except white space.

@maboresev The patch is failing because of automated test for filter admin page is expecting redirection but that is not happening for that test, That test needs to be updated as well.

There was 1 error:

1) Drupal\Tests\filter\Functional\FilterAdminTest::testFilterAdmin
Behat\Mink\Exception\ExpectationException: Current page is "/admin/config/content/formats/manage/restricted_html", but "/admin/config/content/formats" expected.

/var/www/html/vendor/behat/mink/src/WebAssert.php:770
/var/www/html/vendor/behat/mink/src/WebAssert.php:54
/var/www/html/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:435
/var/www/html/core/modules/filter/tests/src/Functional/FilterAdminTest.php:206

ERRORS!
Tests: 5, Assertions: 103, Errors: 1.
ilya.no’s picture

Adding patch with update for tests.

Status: Needs review » Needs work
msankhala’s picture

Here is updated patch with updated test cases. Hopefully this time all the test will pass.

Status: Needs review » Needs work

The last submitted patch, 11: 277054-11.patch, failed testing. View results

msankhala’s picture

Status: Needs work » Needs review
FileSize
5.46 KB
814 bytes

Another attempt to fix error reported by test bot. There is no redirection if you disable a format.

barthje’s picture

Status: Needs review » Reviewed & tested by the community

I do wonder what your use case is that you find it annoying when it goes back to the overview page. But the solution is ok because when you're going there in the normal way it will use the destination parameter.

I've reviewed and tested it and it works like discussed in this issue.

Chi’s picture

It's when you are not actually done with editing the form but want to submit it to debug something or just preserve your changes.

barthje’s picture

Fair enough :)

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 13: 2977054-13.patch, failed testing. View results

msankhala’s picture

Status: Needs work » Reviewed & tested by the community

Looks like issue with the test bot.

larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed as 9b28e1f and pushed to 8.7.x.

larowlan’s picture

Status: Fixed » Reviewed & tested by the community

Having some d.o git issues pushing. Resetting status in meantime

  • larowlan committed 9b28e1f on 8.7.x
    Issue #2977054 by msankhala, ilya.no, veerasekar.r89: Edit text format...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Resolved the issue

Status: Fixed » Closed (fixed)

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