Follow-up of [#194362]

Quoting myself from there:

The new editor entity is only created on the ajax callback/configure button.

If you disable JS, add a new format, select an editor and don't click on "Configure" but directly on save, you get a "Fatal error: Call to undefined method stdClass::save() in .../core/modules/editor/editor.module on line 220 ".

The same will happen if you add a test where you leave out the drupalPostAjax() there.

I'm not sure what's the best thing to do, but you should be able to provoke fatal errors through the UI even if you're doing it "wrong" ;) Either add a validation along the lines of "You must configure the editor" and reload the form with the configuration thingy updated or create the editor entity on demand if it doesn't exist yet.

CommentFileSizeAuthor
#7 1950634-7.patch3.69 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

Priority: Normal » Major

That sounds major...

Berdir’s picture

Well, it's a fatal error yes.

But you can only trigger it if you disable JS and do *not* click on the button that's there right beside the editor selection form but go right to the submit button at the end of the form, so not sure about major :)

webchick’s picture

Right, well that's why major and not critical. :D

Wim Leers’s picture

Title: Fatal error when creating a new format with an editor without JS » Fatal error when creating a new text format with a text editor while JS is disabled
Component: base system » editor.module
Assigned: Unassigned » Wim Leers
Issue tags: +sprint, +Spark

I'll fix this.

dcam’s picture

Assigned: Wim Leers » Unassigned
Issue tags: +Needs issue summary update

http://drupal.org/node/1427826 contains instructions for updating the issue summary with the summary template.

Wim Leers’s picture

Assigned: Unassigned » Wim Leers

I'll fix this this week.

Wim Leers’s picture

Status: Active » Needs review
Issue tags: -Needs tests, -Needs issue summary update
FileSize
3.69 KB

Fixed, with test coverage.

One-line fix, but additional validation and test coverage of course add up. As you (or dawehner) recently said: it's impossible to have one-line Drupal core patches anymore, because then you're missing the regression tests! :)

jessebeach’s picture

Status: Needs review » Reviewed & tested by the community

I manually tested 8.x HEAD to confirm the bug still exists. It does.

I manually tested the patch in #7 with JavaScript disabled. Selecting the CKEditor on the Text Format configuration form without pressing the Configure button before saving the form results in a form validation error rather than a fatal error.

I ran the tests that are introduced in this patch. They pass.

It's ready for commit.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 814089c and pushed to 8.x. Thanks!

Wim Leers’s picture

Issue tags: -sprint +CKEditor in core
Wim Leers’s picture

Issue tags: -CKEditor in core

.

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