#303930: Pluggable architecture for aggregator.module accidentally dropped the form for aggregator_allowed_html_tags. Needs to be brought back.

Will post a patch soon.

CommentFileSizeAuthor
#19 aggregator-allowed-tags.patch1.05 KBmr.baileys
#14 486246-10_aggregator_allowed_html_tags1.patch1.1 KBBerdir
#10 486246-10_aggregator_allowed_html_tags.patch1.08 KBmustafau
#7 486246-7_aggregator_allowed_html_tags.patch1.07 KBmustafau
#1 486246-1_aggregator_allowed_html_tags.patch1.09 KBalex_b
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alex_b’s picture

alex_b CreditAttribution: alex_b commented
Assigned: alex_b » Unassigned
Status: Active » Needs review
FileSize
486246-1_aggregator_allowed_html_tags.patch1.09 KB

Adds aggregator_allowed_html_tags to the top of the aggregator settings form.

aggregator_allowed_html_tags is used for rendering feed descriptions and feed item descriptions and is therefore a global aggregator setting - (as opposed to a fetcher, parser or processor setting).

brianV’s picture

brianV CreditAttribution: brianV commented

Provided it passes the tests, it looks ready to RTBC.

I had noticed that was missing in HEAD. Something I used in D6 - thought there was some great plan at work in it's disappearance.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Is it worth writing tests for this?

Status: Needs review » Needs work

The last submitted patch failed testing.

brianV’s picture

brianV CreditAttribution: brianV commented
Status: Needs work » Needs review

resetting to 'needs review' - testbot was broken.

Status: Needs review » Needs work

The last submitted patch failed testing.

mustafau’s picture

mustafau CreditAttribution: mustafau commented
Status: Needs work » Needs review
FileSize
486246-7_aggregator_allowed_html_tags.patch1.07 KB

Reroll.

brianV’s picture

brianV CreditAttribution: brianV commented
Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

mustafau’s picture

mustafau CreditAttribution: mustafau commented
Status: Needs work » Needs review
FileSize
486246-10_aggregator_allowed_html_tags.patch1.08 KB
brianV’s picture

brianV CreditAttribution: brianV commented
Status: Needs review » Reviewed & tested by the community
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Reviewed & tested by the community » Needs review

This line:

+ $form = array();

is not necessary.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented
Status: Needs review » Needs work

That is correct.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Needs work » Needs review
FileSize
486246-10_aggregator_allowed_html_tags1.patch1.1 KB

Re-roll. I've also removed the brackets in the description. Regarding that, I'm not sure if it would make sense to rewrite that, using the word Drupal "feels" wrong imho.

brianV’s picture

brianV CreditAttribution: brianV commented 
+++ modules/aggregator/aggregator.admin.inc	22 Jan 2010 18:53:22 -0000
@@ -385,6 +385,16 @@ function aggregator_admin_refresh_feed($
+    '#description' => t('A space-separated list of HTML tags allowed in the content of feed items. Tags in this list are not removed by Drupal.'),

Perhaps this?

'Tags in this list are not removed from imported feed items.'

sun.core’s picture

sun.core CreditAttribution: sun.core commented
Priority: Critical » Normal

Not critical. Please read and understand Priority levels of Issues, thanks.

sun.core’s picture

sun.core CreditAttribution: sun.core commented

#14: 486246-10_aggregator_allowed_html_tags1.patch queued for re-testing.

heather’s picture

heather CreditAttribution: heather commented

Subscribing.

As it is, setting up a Feed import with Drupal 7 means you can't allow images to be imported from feeds, or other HTML tags.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys commented
FileSize
aggregator-allowed-tags.patch1.05 KB

Re-rolled to remove some trailing white-space, and reworded the description to:

A space-separated list of HTML tags allowed in the content of feed items. Disallowed tags are stripped from the content.

heather’s picture

heather CreditAttribution: heather commented

Hm... tested on a fresh install, and this is the error I got:

* Notice: Undefined index: aggregator_processors in aggregator_admin_form_submit() (line 501 of /Users/heather/Sites/d7march/drupal7march/modules/aggregator/aggregator.admin.inc).
* Warning: array_filter(): The first argument should be an array in aggregator_admin_form_submit() (line 501 of /Users/heather/Sites/d7march/drupal7march/modules/aggregator/aggregator.admin.inc).

allowed html

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys commented

@heather: true, but those appear both with and without this patch, and are addressed by #464792: Notice/warnings on submitting aggregator settings form

alex_b’s picture

alex_b CreditAttribution: alex_b commented
Status: Needs review » Reviewed & tested by the community

This test fixes a regression introduced with D7 development. Only difference is the wording of the description introduced in #14 which is much better now IMO:

D6: A space-separated list of HTML tags allowed in the content of feed items. (Tags in this list are not removed by Drupal.)
D7: A space-separated list of HTML tags allowed in the content of feed items. Disallowed tags are stripped from the content.

RTBC.

realityloop’s picture

realityloop
he/him
CreditAttribution: realityloop commented

#19: aggregator-allowed-tags.patch queued for re-testing.

ksenzee’s picture

ksenzee
she/her
Location Seattle area
CreditAttribution: ksenzee commented

#19: aggregator-allowed-tags.patch queued for re-testing.

ksenzee’s picture

ksenzee
she/her
Location Seattle area
CreditAttribution: ksenzee commented

Confirm this is still RTBC.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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