Problem/Motivation

The filter tips page is broken due to aggressive filtering of the markup on the page.

Proposed resolution

Display the page in full HTML glory.

Remaining tasks

Provide a patch.

User interface changes

Fixes the page.

API changes

None.

CommentFileSizeAuthor
#11 Still_with_errors.png47.48 KBibonelli
#11 2388299-11.patch2.12 KBibonelli
#11 interdiff-2388299-3-11.txt2.26 KBibonelli
#9 interdiff-2388299-3-9.txt779 bytesibonelli
#3 fixed2.png29.04 KBccuta
#3 fixed.png20.38 KBccuta
#3 error.png19.52 KBccuta
#3 error2.png37.06 KBccuta
#1 2388299-fix-filter-tips-page.patch2.15 KBsam152

Comments

sam152’s picture

sam152 commented
Status: Needs work » Needs review
StatusFileSize
new 2388299-fix-filter-tips-page.patch2.15 KB
prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented

#1 patch is applied successfully.

ccuta’s picture

ccuta commented
Issue summary: View changes
Issue tags: +LatinAmerica2015
StatusFileSize
new error2.png37.06 KB
new error.png19.52 KB
new fixed.png20.38 KB
new fixed2.png29.04 KB

The patch seems to work, it solves the issue.

I'm adding screenshots.

santiago.castro’s picture

santiago.castro commented

This bug is happening as well at the node revision table here: https://www.drupal.org/node/2004702

andrefy’s picture

andrefy commented

#1 patch works for me as well

santiago.castro’s picture

santiago.castro commented
Status: Needs review » Reviewed & tested by the community

Tested several times, the bug is still hapenning

jackbravo’s picture

jackbravo commented

Even though the last comment is a little confusing. I think he means that the bug is on drupal head, but he marked it RTBC because the fix indeed fixes the issue.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work

So while this looks like it fixes the bug (great!), and it even copies patterns used many times in core, in general we're not supposed to u se SafeMarkup::set() wherever possible, in favour of proper theme layer stuff. See https://www.drupal.org/node/2296163 for more details.

EDIT: Now with a *non* recursive link back to this same issue. ;)

ibonelli’s picture

ibonelli commented
StatusFileSize
new interdiff-2388299-3-9.txt779 bytes

I'm attaching an interdiff of what I'm doing. I'm going to upload the full patch soon. According to webchick comments this should be ok, but please chime in if it doesn't feel ok.

idebr’s picture

idebr commented
Related issues: +#2346209: /filter/tips improperly escaped

There is an issue covering the same bug at #2346209: /filter/tips improperly escaped

ibonelli’s picture

ibonelli commented
StatusFileSize
new interdiff-2388299-3-11.txt2.26 KB
new 2388299-11.patch2.12 KB
new Still_with_errors.png47.48 KB

Thanks for the reference. I've been looking at the sibling issue, and the proposed solution has a very different approach. I wonder which one is better?

This issue is working with core/modules/filter/src/Plugin/Filter/FilterHtml.php which seems more relevant to me as it is where (as far as I can tell) the page is being generated. In the sibling issue the solution being applied is part of the module (core/modules/filter/filter.module) which as far as I can tell it is done while returning the data from FilterHtml.php.

I'm attaching a patch that needs work because the original approach whithin this issue which I used to base my work doesn't work when we move from using SafeMarkup::set() to Xss::filter, the Basic HTML error gets fixed, but the table breaks (see attached screenshot).

In any case proposed solution on issue #2346209 seems better to me and we might simply be able to close this as a duplicate?

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Status: Needs work » Closed (duplicate)

There is already an issue for this #2346209: /filter/tips improperly escaped, please make changes to the first one.