Issue status update for http://drupal.org/node/29275 Post a follow up: http://drupal.org/project/comments/add/29275 Project: Drupal Version: cvs Component: aggregator.module Category: feature requests Priority: normal Assigned to: Uwe Hermann Reported by: Uwe Hermann Updated by: Prometheus6 Status: patch (code needs review) +1 I know it works because it's identical to what I did. Prometheus6 Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 00:13:21 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/aggregator_allowed_html_tags.patch (2.34 KB) I'm currently trying to get aggregator.module in shape for being used as a planet-like software for a Planet Drupal [1]. Here's a patch which allows the site admin to specify which HTML tags are stripped from feeds (or not). This is hardcoded in aggregator.module right now, the attached 2-line patch (for HEAD) makes it configurable. I propose to add at least "<img> <font> <blockquote> <div> <span> <code>" to the default list of allowed HTML tags, but I'll leave that for another patch. Any objections? [1] http://drupal.org/node/28194

Issue status update for http://drupal.org/node/29275 Post a follow up: http://drupal.org/project/comments/add/29275 Project: Drupal Version: cvs Component: aggregator.module Category: feature requests Priority: normal Assigned to: Uwe Hermann Reported by: Uwe Hermann Updated by: Bèr Kessels Status: patch (code needs review) sorry. -1. IMHO aggregator should be small, clean and lean. No options, features or whatevers. (node aggregator does this for free, and much more. If you want advanced aggregation, then please add your development to this module, or to another -new- advanced aggregation module) Bèr Kessels Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 00:13:21 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/aggregator_allowed_html_tags.patch (2.34 KB) I'm currently trying to get aggregator.module in shape for being used as a planet-like software for a Planet Drupal [1]. Here's a patch which allows the site admin to specify which HTML tags are stripped from feeds (or not). This is hardcoded in aggregator.module right now, the attached 2-line patch (for HEAD) makes it configurable. I propose to add at least "<img> <font> <blockquote> <div> <span> <code>" to the default list of allowed HTML tags, but I'll leave that for another patch. Any objections? [1] http://drupal.org/node/28194 ------------------------------------------------------------------------ Sun, 21 Aug 2005 03:18:30 +0000 : Prometheus6 +1 I know it works because it's identical to what I did. ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:22:54 +0000 : Junyor This has come up before: http://drupal.org/node/14104. Could you possibly use input filters instead of a setting for this?

Issue status update for http://drupal.org/node/29275 Post a follow up: http://drupal.org/project/comments/add/29275 Project: Drupal Version: cvs Component: aggregator.module Category: feature requests Priority: normal Assigned to: Uwe Hermann Reported by: Uwe Hermann Updated by: robertDouglass Status: patch (code needs review) I have to add that I would rather have this patch than nothing, since as Uwe correctly points out, the behavior is hardcoded and not suitable for many common cases. robertDouglass Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 00:13:21 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/aggregator_allowed_html_tags.patch (2.34 KB) I'm currently trying to get aggregator.module in shape for being used as a planet-like software for a Planet Drupal [1]. Here's a patch which allows the site admin to specify which HTML tags are stripped from feeds (or not). This is hardcoded in aggregator.module right now, the attached 2-line patch (for HEAD) makes it configurable. I propose to add at least "<img> <font> <blockquote> <div> <span> <code>" to the default list of allowed HTML tags, but I'll leave that for another patch. Any objections? [1] http://drupal.org/node/28194 ------------------------------------------------------------------------ Sun, 21 Aug 2005 03:18:30 +0000 : Prometheus6 +1 I know it works because it's identical to what I did. ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:22:54 +0000 : Junyor This has come up before: http://drupal.org/node/14104. Could you possibly use input filters instead of a setting for this? ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:59:41 +0000 : Bèr Kessels sorry. -1. IMHO aggregator should be small, clean and lean. No options, features or whatevers. (node aggregator does this for free, and much more. If you want advanced aggregation, then please add your development to this module, or to another -new- advanced aggregation module) ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:09:28 +0000 : robertDouglass -1 for this approach and a big +1 for running aggregators through filters that can be configured.

Issue status update for http://drupal.org/node/29275 Post a follow up: http://drupal.org/project/comments/add/29275 Project: Drupal Version: cvs Component: aggregator.module Category: feature requests Priority: normal Assigned to: Uwe Hermann Reported by: Uwe Hermann Updated by: Prometheus6 Status: patch (code needs review) Yes, the filter system could be used, but if you really want lean and mean this really minor patch gives you much of the functionality of one of the filters that ships in core without a lot of UI development. It will NEVER make the 4.7 code freeze. I suggest applying this patch for 4.7 and working on using the filter system in the aggregator later. Prometheus6 Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 00:13:21 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/aggregator_allowed_html_tags.patch (2.34 KB) I'm currently trying to get aggregator.module in shape for being used as a planet-like software for a Planet Drupal [1]. Here's a patch which allows the site admin to specify which HTML tags are stripped from feeds (or not). This is hardcoded in aggregator.module right now, the attached 2-line patch (for HEAD) makes it configurable. I propose to add at least "<img> <font> <blockquote> <div> <span> <code>" to the default list of allowed HTML tags, but I'll leave that for another patch. Any objections? [1] http://drupal.org/node/28194 ------------------------------------------------------------------------ Sun, 21 Aug 2005 03:18:30 +0000 : Prometheus6 +1 I know it works because it's identical to what I did. ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:22:54 +0000 : Junyor This has come up before: http://drupal.org/node/14104. Could you possibly use input filters instead of a setting for this? ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:59:41 +0000 : Bèr Kessels sorry. -1. IMHO aggregator should be small, clean and lean. No options, features or whatevers. (node aggregator does this for free, and much more. If you want advanced aggregation, then please add your development to this module, or to another -new- advanced aggregation module) ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:09:28 +0000 : robertDouglass -1 for this approach and a big +1 for running aggregators through filters that can be configured. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:10:44 +0000 : robertDouglass I have to add that I would rather have this patch than nothing, since as Uwe correctly points out, the behavior is hardcoded and not suitable for many common cases. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:36:38 +0000 : Bèr Kessels can we not use the default filter system here?

Issue status update for http://drupal.org/node/29275 Post a follow up: http://drupal.org/project/comments/add/29275 Project: Drupal Version: cvs Component: aggregator.module Category: feature requests Priority: normal Assigned to: Uwe Hermann Reported by: Uwe Hermann Updated by: Prometheus6 Status: patch (code needs review) Clarification: Adapting the aggregator to use the filter system is a big enough job that doing so in time for the code freeze is unlikely. Seriously, how important is is? It HAS been raised before... Meanwhile you get a significant improvement for two lines of code. There is literally no down side to applying this patch that I can see...I really don't understand why it would NOT be applied. Prometheus6 Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 00:13:21 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/aggregator_allowed_html_tags.patch (2.34 KB) I'm currently trying to get aggregator.module in shape for being used as a planet-like software for a Planet Drupal [1]. Here's a patch which allows the site admin to specify which HTML tags are stripped from feeds (or not). This is hardcoded in aggregator.module right now, the attached 2-line patch (for HEAD) makes it configurable. I propose to add at least "<img> <font> <blockquote> <div> <span> <code>" to the default list of allowed HTML tags, but I'll leave that for another patch. Any objections? [1] http://drupal.org/node/28194 ------------------------------------------------------------------------ Sun, 21 Aug 2005 03:18:30 +0000 : Prometheus6 +1 I know it works because it's identical to what I did. ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:22:54 +0000 : Junyor This has come up before: http://drupal.org/node/14104. Could you possibly use input filters instead of a setting for this? ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:59:41 +0000 : Bèr Kessels sorry. -1. IMHO aggregator should be small, clean and lean. No options, features or whatevers. (node aggregator does this for free, and much more. If you want advanced aggregation, then please add your development to this module, or to another -new- advanced aggregation module) ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:09:28 +0000 : robertDouglass -1 for this approach and a big +1 for running aggregators through filters that can be configured. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:10:44 +0000 : robertDouglass I have to add that I would rather have this patch than nothing, since as Uwe correctly points out, the behavior is hardcoded and not suitable for many common cases. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:36:38 +0000 : Bèr Kessels can we not use the default filter system here? ------------------------------------------------------------------------ Sun, 21 Aug 2005 14:38:28 +0000 : Prometheus6 Yes, the filter system could be used, but if you really want lean and mean this really minor patch gives you much of the functionality of one of the filters that ships in core without a lot of UI development. It will NEVER make the 4.7 code freeze. I suggest applying this patch for 4.7 and working on using the filter system in the aggregator later. ------------------------------------------------------------------------ Sun, 21 Aug 2005 15:00:44 +0000 : Morbus Iff -1. Filters.

Issue status update for http://drupal.org/node/29275 Post a follow up: http://drupal.org/project/comments/add/29275 Project: Drupal Version: cvs Component: aggregator.module Category: feature requests Priority: normal Assigned to: Uwe Hermann Reported by: Uwe Hermann Updated by: Morbus Iff Status: patch (code needs review) "In time for the code freeze" is the wrong justification. Forcing a patch through just so it can beat the code freeze allows quick and dirty code, as opposed to the right code. Encouraging quick patches allows the least tested and least designed code to get in during the point when the code should be most stable and eyeballed. I'd much rather the right code than a bunch of quick patches that do things half-assed (in design, not quality - your patch is just fine for what it attempts to do, just not the best way to do it). Morbus Iff Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 00:13:21 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/aggregator_allowed_html_tags.patch (2.34 KB) I'm currently trying to get aggregator.module in shape for being used as a planet-like software for a Planet Drupal [1]. Here's a patch which allows the site admin to specify which HTML tags are stripped from feeds (or not). This is hardcoded in aggregator.module right now, the attached 2-line patch (for HEAD) makes it configurable. I propose to add at least "<img> <font> <blockquote> <div> <span> <code>" to the default list of allowed HTML tags, but I'll leave that for another patch. Any objections? [1] http://drupal.org/node/28194 ------------------------------------------------------------------------ Sun, 21 Aug 2005 03:18:30 +0000 : Prometheus6 +1 I know it works because it's identical to what I did. ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:22:54 +0000 : Junyor This has come up before: http://drupal.org/node/14104. Could you possibly use input filters instead of a setting for this? ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:59:41 +0000 : Bèr Kessels sorry. -1. IMHO aggregator should be small, clean and lean. No options, features or whatevers. (node aggregator does this for free, and much more. If you want advanced aggregation, then please add your development to this module, or to another -new- advanced aggregation module) ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:09:28 +0000 : robertDouglass -1 for this approach and a big +1 for running aggregators through filters that can be configured. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:10:44 +0000 : robertDouglass I have to add that I would rather have this patch than nothing, since as Uwe correctly points out, the behavior is hardcoded and not suitable for many common cases. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:36:38 +0000 : Bèr Kessels can we not use the default filter system here? ------------------------------------------------------------------------ Sun, 21 Aug 2005 14:38:28 +0000 : Prometheus6 Yes, the filter system could be used, but if you really want lean and mean this really minor patch gives you much of the functionality of one of the filters that ships in core without a lot of UI development. It will NEVER make the 4.7 code freeze. I suggest applying this patch for 4.7 and working on using the filter system in the aggregator later. ------------------------------------------------------------------------ Sun, 21 Aug 2005 15:00:44 +0000 : Morbus Iff -1. Filters. ------------------------------------------------------------------------ Sun, 21 Aug 2005 15:03:49 +0000 : Prometheus6 Clarification: Adapting the aggregator to use the filter system is a big enough job that doing so in time for the code freeze is unlikely. Seriously, how important is is? It HAS been raised before... Meanwhile you get a significant improvement for two lines of code. There is literally no down side to applying this patch that I can see...I really don't understand why it would NOT be applied. ------------------------------------------------------------------------ Sun, 21 Aug 2005 15:09:12 +0000 : Uwe Hermann I agree with Prometheus6. The patch changes absolutely no functionality in any way, it simply makes something configurable which is hard-coded right now. I'd rather have this in 4.7. than nothing.

Issue status update for http://drupal.org/node/29275 Post a follow up: http://drupal.org/project/comments/add/29275 Project: Drupal Version: cvs Component: aggregator.module Category: feature requests Priority: normal Assigned to: Uwe Hermann Reported by: Uwe Hermann Updated by: robertDouglass Status: patch (code needs review) Well, I'm happy to agree with everyone here. Let's apply the patch the Uwe has provided, as he can then turn around and do planet drupal, and we don't forget that the right way to do this is with filters. robertDouglass Previous comments: ------------------------------------------------------------------------ Sun, 21 Aug 2005 00:13:21 +0000 : Uwe Hermann Attachment: http://drupal.org/files/issues/aggregator_allowed_html_tags.patch (2.34 KB) I'm currently trying to get aggregator.module in shape for being used as a planet-like software for a Planet Drupal [1]. Here's a patch which allows the site admin to specify which HTML tags are stripped from feeds (or not). This is hardcoded in aggregator.module right now, the attached 2-line patch (for HEAD) makes it configurable. I propose to add at least "<img> <font> <blockquote> <div> <span> <code>" to the default list of allowed HTML tags, but I'll leave that for another patch. Any objections? [1] http://drupal.org/node/28194 ------------------------------------------------------------------------ Sun, 21 Aug 2005 03:18:30 +0000 : Prometheus6 +1 I know it works because it's identical to what I did. ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:22:54 +0000 : Junyor This has come up before: http://drupal.org/node/14104. Could you possibly use input filters instead of a setting for this? ------------------------------------------------------------------------ Sun, 21 Aug 2005 08:59:41 +0000 : Bèr Kessels sorry. -1. IMHO aggregator should be small, clean and lean. No options, features or whatevers. (node aggregator does this for free, and much more. If you want advanced aggregation, then please add your development to this module, or to another -new- advanced aggregation module) ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:09:28 +0000 : robertDouglass -1 for this approach and a big +1 for running aggregators through filters that can be configured. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:10:44 +0000 : robertDouglass I have to add that I would rather have this patch than nothing, since as Uwe correctly points out, the behavior is hardcoded and not suitable for many common cases. ------------------------------------------------------------------------ Sun, 21 Aug 2005 09:36:38 +0000 : Bèr Kessels can we not use the default filter system here? ------------------------------------------------------------------------ Sun, 21 Aug 2005 14:38:28 +0000 : Prometheus6 Yes, the filter system could be used, but if you really want lean and mean this really minor patch gives you much of the functionality of one of the filters that ships in core without a lot of UI development. It will NEVER make the 4.7 code freeze. I suggest applying this patch for 4.7 and working on using the filter system in the aggregator later. ------------------------------------------------------------------------ Sun, 21 Aug 2005 15:00:44 +0000 : Morbus Iff -1. Filters. ------------------------------------------------------------------------ Sun, 21 Aug 2005 15:03:49 +0000 : Prometheus6 Clarification: Adapting the aggregator to use the filter system is a big enough job that doing so in time for the code freeze is unlikely. Seriously, how important is is? It HAS been raised before... Meanwhile you get a significant improvement for two lines of code. There is literally no down side to applying this patch that I can see...I really don't understand why it would NOT be applied. ------------------------------------------------------------------------ Sun, 21 Aug 2005 15:09:12 +0000 : Uwe Hermann I agree with Prometheus6. The patch changes absolutely no functionality in any way, it simply makes something configurable which is hard-coded right now. I'd rather have this in 4.7. than nothing. ------------------------------------------------------------------------ Sun, 21 Aug 2005 15:12:29 +0000 : Morbus Iff "In time for the code freeze" is the wrong justification. Forcing a patch through just so it can beat the code freeze allows quick and dirty code, as opposed to the right code. Encouraging quick patches allows the least tested and least designed code to get in during the point when the code should be most stable and eyeballed. I'd much rather the right code than a bunch of quick patches that do things half-assed (in design, not quality - your patch is just fine for what it attempts to do, just not the best way to do it). ------------------------------------------------------------------------ Sun, 21 Aug 2005 15:23:20 +0000 : Prometheus6 Is there dirty code in the patch? Isn't that kind of theoretical? You don't apply the patch, no gain, no loss. You apply the patch, one gain, no loss. That's my opinion. I'm done.