The image button does not work unless width+height attributes are allowed, even though those are optional [#2626106]

Steps to Reproduce:

First you create a custom Text Format (Test Format) without the Image button: By going to Configuration -> Text formats and editors -> click on “+ Add text format” then enter the required information and Select CKEditor as the “Text Editor”. On the “Active toolbar” remove the Image button by dragging and dropping out of the “Active toolbar” on to the “Available buttons”. Press “Save configuration” when all options are set. (Slide1.png)
Test out that the Image button doesn’t show up on the CKEditor by going to add a new page and selecting the custom Text Format (Test Format): By going to Content -> click on “+ Add content ” -> Page and selecting the “Test Format” as the Text Format for the body (Slide2.png)
Now edit the custom Text Format (Test Format) to add the Image button: By going to Configuration -> Text formats and editors -> click on “Configure” for the custom Text Format (Test Format). On the “Active toolbar” add the Image button by dragging and dropping from the “Available buttons” on to the “Active toolbar”. Press “Save configuration” to save the change. (Slide3.png)
When testing out that the Image button should show up on the CKEditor, it’s not showing up on the Toolbox. By going to Content -> click on “+ Add content ” -> Page and selecting the “Test Format” as the Text Format for the body (Slide4.png)
If you go back to check the custom Text Format (Test Format), the Image button is still there, but it’s not showing up on the CKEditor Toolbox (Slide5.png)

Behavior expected:

On a custom Text Format, we did not add the “Image” buttons as one of the options initially. When trying to add it later on to this custom Text Format, the “Image” button should show up on the CKEditor Toolbox for this Text Format.

What happened instead:

The “Image” button is only showing up on the admin page for the custom Text Format, but it’s not showing up on the CKEditor Toolbox for this Text Format.

We are using:

Drupal core 8.0.0
Standard Profile

Comment	File	Size	Author
#19	2626106-19.patch	882 bytes	wim leers
#5	TextFormatConfiguration.txt	1.08 KB	gceja
#5	TextEditorConfiguration.txt	1.01 KB	gceja
	Slide5.png	283.2 KB	gceja
	Slide4.png	206.22 KB	gceja
	Slide3.png	288.06 KB	gceja
	Slide2.png	207.89 KB	gceja
	Slide1.png	273.45 KB	gceja

Comments

Comment #1

30 November 2015 at 23:19

gceja created an issue. See original summary.

Comment #2

gceja commented 1 December 2015 at 00:33

We tried adding a new custom Text Format with the "Image" button included from the get go and the same behavior is being observed:
The “Image” button is only showing up on the configuration page for the custom Text Format, but it’s not showing up on the CKEditor Toolbox for this Text Format.

Comment #3

gceja commented 1 December 2015 at 00:53

After further testing, we discovered that the "Limit allowed HTML tags and correct faulty HTML" Enabled filter is the one that is preventing the Image button from showing up in the CKEditor Toolbox for the custom Text Format; even though the image tag is included in the "Allowed HTML tags" box:
<img src alt height width data-align data-caption>

Is this behavior that was intended with the "Limit allowed HTML tags and correct faulty HTML", to hide the Image button in the CKEditor Toolbox for the custom Text Format?

Comment #4

wim leers

Ghent 🇧🇪🇪🇺

commented 1 December 2015 at 14:09

Status:

Active

» Postponed (maintainer needs more info)

Please export your text format and text editor's configuration, so I can try to reproduce this.

#3 is correct: if the <img> tag is whitelisted along with those attributes, it should show up.

Comment #5

gceja commented 1 December 2015 at 16:26

Status	File	Size
new	TextEditorConfiguration.txt	1.01 KB
new	TextFormatConfiguration.txt	1.08 KB

Here are the text format and text editor's configuration exports. Please let us know if you need additional information.
Thank you for your time.

Comment #6

wim leers

Ghent 🇧🇪🇪🇺

commented 1 December 2015 at 16:45

Assigned:	Unassigned	» wim leers
Status:	Postponed (maintainer needs more info)	» Active
Issue tags:		+Needs issue summary update

Thanks! Will try to reproduce.

Comment #7

thpoul commented 8 December 2015 at 20:37

@wimleers I was able to reproduce on a clean 8.0.x by importing the configuration provided at #5.

I tried debugging and found out that on admin/config/content/formats/manage/test_format Line 245 of core/modules/filter/filter.filter_html.admin.js gives an error caught TypeError: Cannot read property 'tagName' of null.

I will try investigating further.

Comment #8

thpoul commented 8 December 2015 at 20:57

Looks like the error was not relevant but still must be investigated. The configuration was missing the data-entity-type data-entity-uuid attributes.

Changing <img src alt height width data-align data-caption> to <img src alt height width data-entity-type data-entity-uuid data-align data-caption> I am able to see the the drupalimage icon on the editor.

@gceja did you remove the data-entity-type data-entity-uuid attributes while creating the text format?

Comment #9

gceja commented 8 December 2015 at 21:50

We updated the "Allowed HTML tags" to include the two attributes that @thpoul mentioned: data-entity-type data-entity-uuid
And the Image button is now showing up on the CKEditor toolbox, thanks for looking into it.

To answer your question: did you remove the data-entity-type data-entity-uuid attributes while creating the text format?
The image tag was updated and set without those two attributes when we migrated to RC1, now we are on 8.0.1 and those attributes show up correctly as defaults.

Comment #10

thpoul commented 8 December 2015 at 22:38

Priority:	Major	» Normal
Issue tags:	-Needs issue summary update	+Needs subsystem maintainer review

@gceja Great to see that you got it resolved!

@wimleers should this considered as a non-issue and be closed? Tagging accordingly.

Comment #11

jepster_

🇩🇪Düsseldorf

commented 4 January 2016 at 12:18

Confirming #3:

After further testing, we discovered that the "Limit allowed HTML tags and correct faulty HTML" Enabled filter is the one that is preventing the Image button from showing up in the CKEditor Toolbox for the custom Text Format;

Comment #12

thpoul commented 4 January 2016 at 20:18

@jepSter Could it be the same case as #9? If not, could you please you provide more information on how to replicate and export your text format and text editor's configuration?

Comment #13

anon

Swedish

Gothenburg

commented 9 February 2016 at 19:04

Version:

8.0.0

» 8.0.3

Same issue for me. The icon is present in the toolbar, but when I click on it, nothing happens. No JS errors either.

Have <img src alt data-entity-type data-entity-uuid>in the Allowed HTML tags.

The only way to enable the image modal is to uncheck Limit allowed HTML tags and correct faulty HTML

Comment #14

bburg

he/him

Washington, DC

commented 14 February 2016 at 17:27

I believe I was encountering a very similar issue, or likely the same with some variation.

Create a custom text format with the image plugin enabled.
Go to add a node with the text format enabled.
Switch to the custom text format (I had set it to be the default, but the node still reverts to Basic HTML. A different issue I guess?)
Image icon is showing, but clicking on it does nothing.

No js errors in the console. Clicking the plugin does appear to make some requests to some cryptic url (data:image/gif;base64,R0lGODlhAQABAPABAP///wAAACH5BAEKAAAALAAAAAABAAEAAAICRAEAOw==), I have no idea what this is.

~~Updating from 8.0.2 to 8.0.3 seems to have fixed this.~~

Edit: This is still broken in 8.0.3. I had disabled the "Limit allowed HTML tags and correct faulty HTML" filter while trying out a few things.

Comment #15

inaetaru commented 21 February 2016 at 15:55

I'm having similar issue;

New installation, 8.0.3
Create new format with CKEditor, enable image, enable "Allowed HTML tags" filter (and make sure IMG is allowed)
Clicking on insert image button during editing does nothing, no error.
If I insert image manually to the source, it's displayed.
(WYSIWYG mode) If I right-click on manually inserted image and select image properties, nothing happens. No error.

Comment #16

jon@s commented 25 February 2016 at 19:49

Same issue here (8.0.0 and 8.0.3). but it worked when I added width and height to the img in allowed tags, see if that works for you.

Comment #17

inaetaru commented 27 February 2016 at 16:49

@Jon@s #16 That fixed my issue, thank you very much!

Comment #18

jon@s commented 27 February 2016 at 18:23

Added related issue Image dialog on custom text format with limit html doesn't work [2677142] under editor.module which I believe is the root of the problem.

Comment #19

wim leers

Ghent 🇧🇪🇪🇺

commented 4 March 2016 at 12:58

Title:	Adding the "Image" button on CKEditor Text Format	» The image button does not work unless width+height attributes are allowed, even though those are optional
Version:	8.0.3	» 8.0.x-dev
Status:	Active	» Needs review
Issue tags:	-Needs subsystem maintainer review
Related issues:		+#2519862: Remove the width/height inputs from EditorImageDialog

Status	File	Size
new	2626106-19.patch	882 bytes

This was a small oversight in #2519862: Remove the width/height inputs from EditorImageDialog. Which never really was a problem, until #2549077: Allow the "Limit allowed HTML tags" filter to also restrict HTML attributes, and only allow a small whitelist of attributes by default landed a few months later. It's the combination of those two that caused this problem.

Fix attached: it's simply the drupalimage CKEditor plugin whose requiredContent setting requiring too much.

Comment #20

wim leers

Ghent 🇧🇪🇪🇺

commented 4 March 2016 at 12:58

Comment #21

duaelfr

he/him

French

Montpellier, France

commented 4 March 2016 at 13:09

RTBC for me :)
I let @thpoul change the status if he agrees.

Comment #22

wim leers

Ghent 🇧🇪🇪🇺

commented 4 March 2016 at 13:11

Issue tags:

+Needs manual testing

Let's be more clear.

@DuaelFr, did you do manual testing?

Comment #23

wim leers

Ghent 🇧🇪🇪🇺

commented 4 March 2016 at 13:11

Issue tags:

+JavaScript

Comment #24

duaelfr

he/him

French

Montpellier, France

commented 4 March 2016 at 13:14

Issue tags:

-Needs manual testing

Yes, I did :)
I reproduced the issue then patched Core to see it fixed.
Reviewed code too (not so long ^^).

Comment #25

wim leers

Ghent 🇧🇪🇪🇺

commented 4 March 2016 at 13:23

Perfect :)

Comment #26

thpoul commented 4 March 2016 at 13:39

Status:

Needs review

» Reviewed & tested by the community

Was waiting for the test to turn green to comment. I can confirm the patch works by manual testing it and by #21 changing the status to RTBC

Comment #27

jepster_

🇩🇪Düsseldorf

commented 7 March 2016 at 09:06

Thank you guys.

Comment #28

9 March 2016 at 00:14

catch committed 73dddd4 on 8.2.x

Issue #2626106 by Wim Leers, gceja, thpoul: The image button does not...

Comment #29

9 March 2016 at 00:14

catch committed 740b838 on 8.1.x

Issue #2626106 by Wim Leers, gceja, thpoul: The image button does not...

Comment #30

9 March 2016 at 00:14

catch committed 79a5090 on 8.0.x

Issue #2626106 by Wim Leers, gceja, thpoul: The image button does not...

Comment #31

catch

he/him

English

commented 9 March 2016 at 00:14

Status:

Reviewed & tested by the community

» Fixed

Committed/pushed to all three 8.x branches, thanks!

Comment #32

23 March 2016 at 00:14

Status:

Fixed

» Closed (fixed)

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

The image button does not work unless width+height attributes are allowed, even though those are optional

Steps to Reproduce:

Behavior expected:

What happened instead:

We are using:

Comments

Related issues

Referenced by