There's an unnecessary span (.source) around an anchor. The span can be removed and its class moved to the anchor element.
https://skitch.com/jesse.beach/fkkfk/drupal-dev-netbeans-ide-7.0

The classes should be moved into a template variable.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aspilicious’s picture

Status: Active » Needs review
FileSize
2.14 KB

Something like this?

Status: Needs review » Needs work

The last submitted patch, aggregator-item.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
2.09 KB

Reconfiguring text editor. Lets see..

Status: Needs review » Needs work

The last submitted patch, aggregator-item-unix.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
2.09 KB

hmm, another try

jessebeach’s picture

I removed the code in the template referring to $source_url and $source_title. Looking at the SQL query that drives the data for the items, the field feed_title is neither called from the database for the item:

db_query_range('SELECT i.title, i.timestamp, i.link FROM {aggregator_item} i WHERE i.fid = :fid ORDER BY i.timestamp DESC', 0, variable_get('aggregator_summary_items', 3), array(':fid' => $feed->fid));

nor does it exist as a field in the database for the item

+-------------+--------------+------+-----+---------+----------------+
| Field       | Type         | Null | Key | Default | Extra          |
+-------------+--------------+------+-----+---------+----------------+
| iid         | int(11)      | NO   | PRI | NULL    | auto_increment |
| fid         | int(11)      | NO   | MUL | 0       |                |
| title       | varchar(255) | NO   |     |         |                |
| link        | varchar(255) | NO   |     |         |                |
| author      | varchar(255) | NO   |     |         |                |
| description | longtext     | NO   |     | NULL    |                |
| timestamp   | int(11)      | YES  |     | NULL    |                |
| guid        | varchar(255) | YES  |     | NULL    |                |
+-------------+--------------+------+-----+---------+----------------+

This reduces the complexity of the template down to a simple link and span. I added a call to theme_datetime to render the age of the item as a <time> element.

After making these changes, I started to wonder if the template file is really even necessary. So I made a switch to a theme function for rendering source item summaries. It seems a lot cleaner than a template that gets called often to print two variables.

aspilicious’s picture

I love it and no visual changes.
RTBC for me

cosmicdreams’s picture

Great, simple patch. Will manually test tonight.

dcmouyard’s picture

Status: Needs review » Reviewed & tested by the community

The patch in #6 looks good to me.

cosmicdreams’s picture

Issue tags: +Coding standards

Yep, worked well. Tagging for jhodgdon

jhodgdon’s picture

Issue tags: -Coding standards

Sorry, I don't think this is really just a coding standards patch -- it's doing a lot more than just converting to use HTML5 (restructuring the theme calls). So I'll leave it for Dries/catch to deal with.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Looks great. Committed/pushed to 8.x.

There was one minor issue (double quotes vs. single quotes for the empty space), but I fixed that before commit.

jessebeach’s picture

Great, thanks everyone!

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