This is a followup of #2384487: Make the class variables protected for FilterFormat.

We made the properties of the FilterFormat entity protected. That means that the properties are no longer directly accessible from outside of the class. In the file core/modules/editor/src/Form/EditorImageDialog.php (line 51) we forgot to change the property to a method.

This broke a fundamental part of in-place editing and WYSIWYG editing: you no longer can insert images at all!

CommentFileSizeAuthor
#10 2389381-9.patch2.15 KBswentel
#10 2389381-9-fail.patch1.49 KBswentel
#1 2389381-1.patch701 bytesdaffie
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Active » Needs review
FileSize
2389381-1.patch701 bytes

This is the same patch as https://www.drupal.org/files/issues/2384487-56.patch.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +Needs tests

So we somehow clearly miss some test coverage, right?

daffie’s picture

daffie CreditAttribution: daffie commented

Yes, we do. Wim Leers was not happy that it is impossible to upload or edit images via CKEditor. So I made a patch that hopefully will fix the problem. I did not add test coverage to speed it up. We can add test coverage before we fix the problem. What do you think that the best idea is?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

This issue title is not very useful :) HEAD is broken is usually only used if the test suite is broken, not for "normal" bugs. And the first part should more closely describe what the problem is, like EditorImageDialog tries to access protected property or so.

daffie’s picture

daffie CreditAttribution: daffie commented
Title: Editor_load does not work in EditorImageDialog. HEAD is broken » EditorImageDialog tries to access protected property of FormatFilter class
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Priority: Normal » Critical

This broke a fundamental part of in-place editing and WYSIWYG editing: you no longer can insert images at all! I think that makes this critical.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

So the AJAX error is:

Uncaught AjaxError: 
Path: /editor/dialog/image/basic_html
Fatal error:  Cannot access protected property Drupal\filter\Entity\FilterFormat::$format in /home/sa4d6841a73f658f/www/core/modules/editor/src/Form/EditorImageDialog.php on line 51

Which can probably be used to write a quick test that that URL returns no fatals? :)

swentel’s picture

swentel CreditAttribution: swentel commented
Assigned: daffie » swentel

Quickly checking for a test

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +Ghent DA sprint

.

swentel’s picture

swentel CreditAttribution: swentel commented
Issue tags: -Needs tests
FileSize
2389381-9-fail.patch1.49 KB
2389381-9.patch2.15 KB

Let's see

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented

HAH! Funny regression test, that's not testing much :) If fine for others to unbreak this, I'm fine with it too. But we (I) will need to add proper test coverage for EditorImageDialog at some point.

swentel’s picture

swentel CreditAttribution: swentel commented

Yeah it's a rather naieve one, but the fix is so easy that I'd rather get that one in first, but we'll see what other think.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

I think this is good for a regression test. Let's not block fixing a critical bug on adding elaborate test coverage for things not related to the bug IMHO.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

The last submitted patch, 10: 2389381-9-fail.patch, failed testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Title: EditorImageDialog tries to access protected property of FormatFilter class » Fatal error: EditorImageDialog tries to access protected property of FormatFilter class
Issue summary: View changes
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Title: Fatal error: EditorImageDialog tries to access protected property of FormatFilter class » Impossible to add images in WYSIWYG including in-place editing due to fatal error
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed b665ec3 and pushed to 8.0.x. Thanks!

  • alexpott committed b665ec3 on 8.0.x 
    Issue #2389381 by swentel, daffie: Impossible to add images in WYSIWYG...

Status: Fixed » Closed (fixed)

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