Aggregator module hard coded references to blog module

catch - March 12, 2008 - 18:51
Project:Drupal
Version:6.13
Component:aggregator.module
Category:feature request
Priority:normal
Assigned:Unassigned
Status:closed
Description

Aggregator module displays hard coded links to add blog posts if (module_exists('blog')) which has been around since at least 4.6

This issue came up on Remove blog module from core which I'm going to mark postponed until there's a solution for this.

I can see a couple of ways to deal with this:

*Make the Comment on this news item in your personal blog. link configurable for different node types
* Remove it from the aggregator theme function and implement it in blog.module or somewhere else
* Wait and see happens with aggregator module if there's an effort to get feedapi into core - which might mean a cleaner version of 1.

#1

Shai - March 18, 2008 - 14:57

I believe the functionality provided by the "blog-it" feature is of little use. Removing it will solve the problem that catch raises. For folks who want that feature, it is better to provide as a code snippet for someone to add, and leave it out of core.

It's fired only when a person with blogging rights is on a page in which there are feeds created from the aggregator module. By clicking on an icon you get node/add/blog with the blog item id appended so as to insert the html from the blog post into the body field.

The explanation for this functionality (which actually shows up in the blog module, not the aggregator module where the theme function resides) says:

This actively encourages people to add blog entries about things they see and hear elsewhere in the website and from your syndicated partner sites.

Why this functionality should be removed:

  1. It will solve the problem that catch raises by removing the part of the function that makes a hard-coded reference to the blog module.
  2. Good blogging etiquette dictates that the user should quote relevant material, not paste in the entire post.
  3. Very little convenience is provided. Highlighting relevant text, clicking create content --> blog and then pasting is arguably a better workflow anyway -- and would allow for the removal of this code.
  4. Drupal core uses text-based links, with almost no icons. So this icon really stands out.
  5. The functionality is not configurable via blog or aggregator settings. To turn it off you have to override a theme function. The function can be listed as a snippet in the handbook for folks who want it to add to their template.php. I think providing an easy way for people who want to add it is better than including it by default and forcing people to override the theme function just to turn it off.
  6. The html that gets pasted is really confusing for a blogger who isn't html savvy. Also, since the html plopped in might not be compatible with the input filter, it will likely render in an unexpected way that will be confusing to the blogger. If a video is embedded in the blog post, the code will paste in but the video won't show up if a typical filtered-html input format is used.
  7. This is one small step to try to move D7 to the clean and mean CMS/web framework that we want it to be.

Here is the function in question. It begins on line 889 in aggregator.module:

function theme_aggregator_block_item($item, $feed = 0) {
  global $user;

  $output = '';
  if ($user->uid && module_exists('blog') && user_access('create blog entries')) {
    if ($image = theme('image', 'misc/blog.png', t('blog it'), t('blog it'))) {
      $output .= '<div class="icon">'. l($image, 'node/add/blog', array('attributes' => array('title' => t('Comment on this news item in your personal blog.'), 'class' => 'blog-it'), 'query' => "iid=$item->iid", 'html' => TRUE)) .'</div>';
    }
  }

  // Display the external link to the item.
  $output .= '<a href="'. check_url($item->link) .'">'. check_plain($item->title) ."</a>\n";

  return $output;
}

I propose changing it to:

function theme_aggregator_block_item($item, $feed = 0) {

  // Display the external link to the item.
  $output .= '<a href="'. check_url($item->link) .'">'. check_plain($item->title) ."</a>\n";

  return $output;
}

There is some text in blog.module that describes this feature. That text would also need to be removed.

I'll write a patch for both modules if this thread spurs any interest.

Shai

#2

keith.smith - March 18, 2008 - 15:31

Shai, removing the functionality seems reasonable to me (as you describe) and would move us toward resolution on the larger issue of blog-module-in-core. Sounds good to me.

#3

Shai - March 18, 2008 - 17:01

Here is a feature request from a Sept. 05, with patch, to add "blog-it" functionality to the story module:

http://drupal.org/node/31972

That patch would make the issue at hand even worse. Not that it's going anywhere, but thought I'd reference it. Note the patch includes the use of the quote module to format the quoted blog -- which seeks to remedy one of my complaints.

All in all I think the existence of that issue and the fact it didn't go anywhere is more support that "blog-it" functionality should be outside of Drupal core.

Shai

#4

eigentor - March 18, 2008 - 18:17

+1 for removing it. There is nothing it gives that could not be easily archieved otherwise.

#5

catch - March 19, 2008 - 09:04

Shai, unsurprisingly, +1 from me.

#6

flobruit - March 19, 2008 - 19:37
Status:active» needs review

In my opinion, the blogging etiquette issue is already enough to remove this functionality from core. This mechanisms is also in conflict with the idea of attaching fields to remote content (if fields make it in core) and I think that it doesn't fit with the idea of "infinite interoperability" that Dries mentioned in his keynote speech at DrupalCon.

This functionality is also not very obvious (I've personally never noticed that the "blog" icon had a different url than the aggregator links), so removing it shouldn't have a large impact on existing users. Here's a patch that removes the related code.

The blog.png icon is used by devel_generate.module, so an issue should be posted there if this file is removed.

AttachmentSizeStatusTest resultOperations
aggregator-blog-233407-6.patch4.84 KBIgnoredNoneNone

#7

catch - March 19, 2008 - 20:30

flobruit: thanks for patching this.

I think to get this in, we'd probably need a CHANGELOG.txt entry, and Handbook page with the modified theme function to link to from the 6.x-7.x updates page. Do people think that'd be enough to supply an upgrade path? (although there's no user data here to upgrade of course).

#8

catch - March 19, 2008 - 20:47

http://drupal.org/node/235772 was marked as duplicate, fwiw.

#9

catch - April 15, 2008 - 16:21

Re-roll to keep up with HEAD. Question remains whether this is replaced with a handbook page or very small contrib module.

AttachmentSizeStatusTest resultOperations
blogit.patch4.88 KBIgnoredNoneNone

#10

charly71 - June 28, 2008 - 07:56

If you don't want to change the code try to hide the icon with

.block-aggregator div.icon {display:none}

#11

R.Muilwijk - June 30, 2008 - 11:42

@charly71, why would you want to tell the browser to load an image and then make CSS don't display it? Sounds like a 'hack' to me....

New version which applies to Head attached.

AttachmentSizeStatusTest resultOperations
blogit.patch4.87 KBIgnoredNoneNone

#12

ggevalt - August 5, 2008 - 21:22

Just to throw a different opinion in here....

I think the blog it feature is great. I wish there was an easy way to enable it for the page views (it looks nice in the blocks). Users of our sites use it quite a bit. They blog on a news item and because the blog it feature embeds the news story and link other users coming along have the context of the blogger's opinions.

Yes, you could do this manually.

And yes I could set up the site manually to do this if I were given a patch.

But I think you are talking about a usability feature here. Why take it out just because you don't use it? Why not keep it but make it more user friendly, ie creating the ability to turn it on or off and to associate with whatever content type the admin feels like?

For what it's worth.

cheers

geoff

#13

Dave Reid - September 15, 2008 - 18:04

Agreed that this should be removed from core and available as a contrib module.

#14

mvc - October 31, 2008 - 18:08

This patch works for me, and would save me having to override that theme function for every site which happens to use both modules. I completely agree that the blog module should move to contrib, from whence this sort of feature can be added for anyone who wants it. And hopefully whoever does that will make this feature an option, which is disabled by default.

#15

Dries - November 1, 2008 - 18:23
Status:needs review» fixed

Reviewed, looks perfect. Committed to CVS HEAD. Thanks. :-)

--project followup subject--

Anonymous (not verified) - November 15, 2008 - 18:23

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

#16

Anonymous (not verified) - November 15, 2008 - 18:32
Status:fixed» closed

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

#17

Michael3185 - July 28, 2009 - 22:42
Version:7.x-dev» 6.13
Category:bug report» feature request

[Here is the function in question. It begins on line 889 in aggregator.module]

Many thanks for this. The icons were a real eyesore, and no use to my site users, so I've modified my D6.13 aggregator module and all is well.

 
 

Drupal is a registered trademark of Dries Buytaert.