Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
filter.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
3 Oct 2015 at 02:38 UTC
Updated:
11 Nov 2025 at 09:14 UTC
Jump to comment: Most recent, Most recent file


Comments
Comment #2
droplet commentedComment #3
biguzis commentedrerolled.
Comment #7
biguzis commented#3 first time failed, second time passed. Works on my local. IMO ready for RTBC.
Comment #8
droplet commentedIt's pretty important. In D8, it's very easy to hit this limits. It's not good to fix in contri or wait until 8.1
Comment #9
dawehnerOnce you have a textarea, don't you have to deal with newlines then?
Comment #10
thorandre commentedDid a manual test using filter-chars-limits-2579471-3.patch and it works as expected. Adding screenshots to the summary.
@dawehner about newlines:
Not sure if you're asking about tests, risks or edgecases, but if you're not: I tried to add som linebreaks in the area, and they where removed on save. So it does not seem to be an issue, allthough this is just from a sitebuilders perspective.
Comment #11
thorandre commentedComment #12
thorandre commentedComment #13
swentel commentedDid a quick test just copy/pasting the defaults of this field
That's a lot of tags imo, so tbh, hitting that limit almost brings you to full html anyway, so I'm not sure how much this would help, besides a bit more UX because you'll have faster view on which tags are actually in there. Also, this isn't major at all.
Comment #14
droplet commentedForcing to do bad thing is Critical issue in my mind. Looking at https://www.drupal.org/node/45111, I think it's Critical. I wonder if I should call it BUG now. It tends to be a bug more than task after some thoughts.
Now the CKEditor toolbar config do not sync with bottom tag inputs. When you adding "left / right / center alignment". It more than extra 30 chars per tag.
Case 1:
30 tags * 30 chars = 900 + above 1000 chars = 1900.
Case 2:
above 1000 chars + assumed 5 tags with "class" = 1030 ( Not really edge case, anyone required a little more than default will be hit.. )
You can't use FULL HTML if you blocked any single tags for Security
Comment #15
walangitan commentedI do think that the CKEditor toolbar config not syncing with the allowed HTML filters at the bottom is not ideal. I can see this leading to frustration for users as to why their CKEditor configurations aren't appearing as they would assume if they didn't also include it in an allowed HTML tag. But I tend to agree with swentel here in that the 1024 character limit allows for quite a lot of tags.
I recognize that the "left/right/center" alignment buttons in CKEditor do take up ~30 characters each i.e.
<p class="text-align-center">, but for the default CKEditor setup, they are the largest tags in terms of characters. The others being<sub>, <sup>, <hr />, <table border cellpadding cellspacing style>, <tbody>, <tr>, <td>, <u>,< s>. I would say that the two cases that droplet suggests in comment #14, (1900 characters & 1030 characters) are still edge cases. The 1000 character example from comment #13 is a copy and paste of the same tags, which wouldn't be the case in a real world scenario.For me it is difficult to understand how one would have a case that would require more than the current 1024 characters for allowed tags, but still need to limit tags which is why one couldn't use the FULL_HTML option -- and for that case to be common enough to warrant a change here.
Comment #16
droplet commented@walangitan,
I remember at the time when I posted the issues. I tested with default config with alignment features. So that it may adding more than 30 chars per tags.
---
We need not to find out WHY they hit the limits.
the main problem is: We Will Hit the Limit at Some Point. And We CANNOT use FULL HTML instead because the Security issues.
Comment #17
walangitan commented@droplet - I understand now. I don't see an issue with patch #3 after testing. Looking at how the configurations are stored in the database, there doesn't look to be a limitation for changing from a textfield to a textarea. I would defer to more input from the community on this though.
Comment #18
swentel commentedThis behavior has been there since ages (#254725: Maxlength field for Allowed HTML tags too short and no-one has ever complained about it since then. Let's not focus on security or the state of the issue (because we can discuss that for hours)
Note that I don't oppose the patch, but needs work for two reasons:
We should have a way to disallow entering new lines because it breaks the javascript before submission too.
The real quick fix then (if we want to keep this novice) is to set the maxlength to 2056 for instance.
Comment #19
droplet commented@swentel,
:)
--
We only have advanced attrs config from D8.
There's about 120 HTML elements in HTML5 SPECS and able to fit into 1024 chars. (So It's not a big issue for D7 in most cases)
Thanks for the suggestions :)
Comment #20
walangitan commentedThe patch attached here is for increasing the maxlength to 2056. If we want to move in the other direction (changing the form element to a textarea & not allowing newlines to be saved), we can create that patch as well.
Comment #21
walangitan commentedComment #22
swentel commentedNote - not sure why I came up with 2056 :) 2048 seems more 'logical' because it's the double of 1024 - anyway, works for me.
Comment #23
walangitan commentedUpdated patch to use 2048 :).
Comment #24
walangitan commentedComment #25
jibranAll the feedback from @swentel is addressed so RTBC.
Comment #26
alexpottCan we get a follow to change this to a textarea and to handle the new line issue. I think that that makes a lot of sense.
Committed 988c359 and pushed to 8.0.x and 8.1.x. Thanks!
Comment #29
walangitan commentedFollow up issue for converting this input to a textarea and resolving the newline issue that results from it can be found here.
Comment #30
alexpott