Problem/Motivation

In the JSON Feed v1 spec, it states in the entry for content_html:

Important: the only place HTML is allowed in this format is in content_html.

However, a Drupal user could configure a view (either intentionally or unintentionally) to output HTML for other fields like title.

Proposed resolution

We can either validate this at view configuration time or strip HTML for the appropriate fields at render time. I am leaning towards the latter.

CommentFileSizeAuthor
#8 attributes_that_should-2885052-8.patch8.01 KBmarkdorison
#6 attributes_that_should-2885052-6.patch8.03 KBmarkdorison
#4 no_html_attributes-2885052-4.patch2.65 KBKarlShea
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markdorison created an issue. See original summary.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Issue tags: +Needs tests
KarlShea’s picture

KarlShea
he/him
Primary language English
Location Minneapolis 🇺🇸
CreditAttribution: KarlShea at Weil Street Digital commented

I am leaning towards the latter.

I agree, I think the less configuration/knowledge someone needs to have about the spec the better.

KarlShea’s picture

KarlShea
he/him
Primary language English
Location Minneapolis 🇺🇸
CreditAttribution: KarlShea at Weil Street Digital commented
Status: Active » Needs review
FileSize
no_html_attributes-2885052-4.patch2.65 KB

I'm not sure which direction to go here. I don't like using check_markup(), because that just escapes HTML and I don't think that's what anyone would want. The patch uses strip_tags(), which works, but maybe MailFormatHelper::htmlToText would be better? The problem with that is it looks like it does way more that's mail-specific than would be desired here.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Assigned: Unassigned » markdorison
markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Assigned: markdorison » Unassigned
FileSize
attributes_that_should-2885052-6.patch8.03 KB

@KarlShea I am thinking 'strip_tags' is the best approach.

There are a number of ways to approach this but here is one.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs review » Needs work

Needs re-roll.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Status: Needs work » Needs review
FileSize
attributes_that_should-2885052-8.patch8.01 KB

  • KarlShea committed 964486e on 8.x-1.x authored by markdorison 
    Issue #2885052 by markdorison, KarlShea: Attributes that should not...
KarlShea’s picture

KarlShea
he/him
Primary language English
Location Minneapolis 🇺🇸
CreditAttribution: KarlShea at Weil Street Digital commented
Status: Needs review » Fixed

Looks good

Status: Fixed » Closed (fixed)

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