Problem/Motivation
The <br>
tag should be allowed for the "Basic HTML" text format. Right now, if you use shift+return in CKEditor, a <br>
will be inserted, but will be filtered away upon output due to this configuration mismatch.
Proposed resolution
Allow
in the Basic HTML configuration
Remaining tasks
- Write a patch
- Review
User interface changes
None
API changes
None
Original report by @DjebbZ
Hi
The defaults Filtered HTML format allow for this tags :
<a> <em> <strong> <cite> <blockquote> <code> <ul> <ol> <li> <dl> <dt> <dd>
and it converts line breaks smartly into p
and br
tags.
Which means written p
and br
tags are not interpreted !
So better defaults for Allowed HTML should be :
<a> <em> <strong> <cite> <blockquote> <code> <ul> <ol> <li> <dl> <dt> <dd> <p> <br />
I tested this and it works for both <br />
and <br>
.
Here's a pretty simple patch for the filter.module (My first ever !)
Beta phase evaluation
Issue priority | Normal because it does not affect any APIs |
---|---|
Unfrozen changes | Unfrozen because it improves the usability of WYSIWYG formatted text. |
Prioritized changes | The main goal of this issue is usability. |
Disruption | Not disruptive |
Comment | File | Size | Author |
---|---|---|---|
#12 | 818616-12.patch | 2.98 KB | idebr |
#12 | interdiff-12-9.txt | 1.06 KB | idebr |
#12 | 818616-12.fail_.patch | 2.24 KB | idebr |
#9 | interdiff-9-7.txt | 1.19 KB | idebr |
#9 | 818616-9.patch | 1.92 KB | idebr |
Comments
Comment #1
sunUnfortunately, feature freeze was more than 12 months ago. I'm sorry. Let's revisit this for D8.
Comment #2
nesta_ CreditAttribution: nesta_ commentedAny information about this?
Comment #3
Wim LeersMost of this has long been fixed: see https://drupal.org/node/1941642.
But, the
<br>
tag should indeed also be allowed for the "Basic HTML" text format. Right now, if you use shift+return in CKEditor, a<br>
will be inserted, but will be filtered away upon output due to this configuration mismatch.Comment #4
Bojhan CreditAttribution: Bojhan commentedlol, thats funny. Ok lets do this :)
Comment #5
sunThe HTML filter had problems with unclosed
<br>
as well as self-closing<br/>
tags in the past, so this should come with tests.We should also double-check the test coverage for the paragraph filter, as well as the combination of HTML filter + paragraph filter.
Comment #6
Bojhan CreditAttribution: Bojhan commentedAhh good point, tests.
Comment #7
mgiffordStill needs tests, but the patch didn't apply.
Comment #8
jhedstromStill needs tests.
Comment #9
idebr CreditAttribution: idebr commentedI added a test to check if the
<br>
tag as well as the<br />
tag pass the HTML filter correctly.Re: sun #9
Both the paragraph filter as well as the HTML filter have individual unit tests, but I could not find any tests for a combination of filters. Does your remark still apply? If so, could you elaborate on what should be tested?
Comment #10
idebr CreditAttribution: idebr commentedUnassigned Wim Leers
Comment #11
jhedstromThanks for writing tests ~idebr! Could you perhaps upload a patch that contains *just* the test to illustrate the current failing behavior?
Do folks know if this would still be accepted for 8.0? Seems like a nice usability enhancement, which is allowed I think.
Comment #12
idebr CreditAttribution: idebr commented@jhedstrom The new tests only test the filtering on
<br>
tags actually applies correctly per suggestion of sun in #5. I have added an additional test for the additional configuration in the Standard profile.I have also updated the issue summary to reflect the contents of the patch and included a beta evaluation to explain this is a usability improvement.
Comment #13
idebr CreditAttribution: idebr commentedFixed a typo in the beta evaluation.
Comment #15
jhedstromThis now has tests, and as per the beta phase evaluation, it should be allowed in 8.0 as it resolves usability issue with the WYSIWYG.
Comment #16
Wim LeersNice! :)
Comment #17
alexpottThis is a usability bug. Committed f9821ed and pushed to 8.0.x. Thanks!
Thank you for adding the beta evaluation to the issue summary.
Comment #20
Gábor HojtsyThanks, removing from UX sprint now.