minor changes I just remembered

[Anson]

Hi,

I just upgraded s9y to 1.5.1, and in doing so rediscovered some minor changes that I made around the various files over time. Most of them, it seems, should be considered for mainline inclusion.

• I've added a lastBuildDate line to templates/default/feed_2.0.tpl ("<lastBuildDate>{$metadata.lastBuildDate}</lastBuildDate>") and added lastBuildDate to the metadata in rss.php ("'lastBuildDate' => date(DATE_RSS)"). This was a change suggested by Google's automated feed validator, since s9y RSS uses the Slash module of extra feed data.
• I've replaced <content:encoded> with <description>. The complete lack of real RSS standards makes me cry, and doing this is supposed to be wrong, but more RSS readers seem to handle it than content:encoded.
• In templates/bulletproof/index.tpl, I've added '<link rel="icon" href="{$serendipityBaseURL}favicon.ico" />' after the similar line using "shortcut icon." This is because there is a standard way, and an IE way, to specify your favicon, and to work everywhere you have to do both. See http://en.wikipedia.org/wiki/Favicon
• In templates/default/admin/index.tpl, I added favicon links for consistency. This isn't as big a deal and I don't have a strong opinion on whether it merits inclusion in the main s9y branch.

One other thing worth mentioning is that I think favicons should be a configuration option of bulletproof, not something that require editing a template. Editing a template causes your installation to fail integrity checks. I've fixed it for myself, but it seems like something that shouldn't require fixing.

[garvinhicking]

Hi!

That sounds great! Would you be able to provide a Diff/patch file for this, so I could easily include your changes?

The "content:encoded" change however would possibly break things that currently work well, so I'm a bit reluctant. Since content:encoded is according to spec, do you have any specific clients that do not work with this? I'd like to investigate the issue in depth before making things worse )

I agree that needing to change the index.tpl to make the favicon-change is not so great...I'm not sure yet though if we should forcibly check for "favicon.ico", or if a custom string/input is really better?

Regards,
Garvin

[Anson]

That sounds great! Would you be able to provide a Diff/patch file for this, so I could easily include your changes?

Yeah, I'll try to put that together tonight.

The "content:encoded" change however would possibly break things that currently work well, so I'm a bit reluctant. Since content:encoded is according to spec, do you have any specific clients that do not work with this? I'd like to investigate the issue in depth before making things worse )

I don't remember any longer what particular clients I was having problems with... but actually, here's the w3c recommendation:
http://validator.w3.org/feed/docs/warni ... ntent.html
and the RSS Board echoes the thought:
http://www.rssboard.org/rss-profile#nam ... nt-encoded
So basically it looks like s9y could do it better, but my current patch isn't right either.

I agree that needing to change the index.tpl to make the favicon-change is not so great...I'm not sure yet though if we should forcibly check for "favicon.ico", or if a custom string/input is really better?

Maybe one template option to use a favicon, and one to specify a custom favicon URL?

[yellowled]

Maybe one template option to use a favicon, and one to specify a custom favicon URL?

I think this is actually why we originally opted for the "edit your index.tpl" option -- to not overload BP's theme options. Maybe Don remembers this better than me, the favicon stuff was really his "baby"

YL

[Don Chambers]

Like Yellowled, I don't recall the specifics as to why we adopted the approach we did instead of making it a template option.

If this were to be an option, it would need to be divided up into the different stylesheets - one for IE, and a different method for all other browsers. This is very possible - we already have IE specific stylesheets.

Then there is the issue of providing input for the actual string... so the image format can be specified.

Here is my opinion: The method we are using is the IE method, but is also recognized by all other major browsers (AFAIK). Also, most modern browsers automatically detect the favicon if it exists in the website root folder AND has a file extension of .ico. If someone really wants to change it, be my guest. I don't really see the need. If someone wants to uncomment the line that emits it, they can also modify if necessary for the graphic format.

[Anson]

If this were to be an option, it would need to be divided up into the different stylesheets - one for IE, and a different method for all other browsers. This is very possible - we already have IE specific stylesheets.

I'm not sure I understand what you mean. Which stylesheet needs to be modified on a per-browser basis to support a favicon link in the template?

Then there is the issue of providing input for the actual string... so the image format can be specified.

True. Perhaps the best way to handle this is to require it to be a file in the media library, for which s9y already knows MIME type? I think that would be the most user-friendly configuration option; enable a custom favicon from the media library, or do nothing.

Here is my opinion: The method we are using is the IE method, but is also recognized by all other major browsers (AFAIK).

No, it's not; I provided a link to Wikipedia in my original post which describes the whole situation. In brief, after IE introduced favicons, a standard that conformed to W3C best practices was created. Most browsers use the standard, IE uses the form it originated. I specifically tested this, which is what led to me using both forms in my template.

Also, most modern browsers automatically detect the favicon if it exists in the website root folder AND has a file extension of .ico. If someone really wants to change it, be my guest. I don't really see the need. If someone wants to uncomment the line that emits it, they can also modify if necessary for the graphic format.

Yes, if /favicon.ico exists, and is the favicon you want, and you're fine with URL squatting, then you don't need to do anything. That doesn't describe all the possible situations, however, including mine.

I haven't had time to put together a diff for the other stuff yet, and I'm going to be busy for a couple of days... but I'll see if I can put together the code to show what I mean this weekend.

[yellowled]

If this were to be an option, it would need to be divided up into the different stylesheets - one for IE, and a different method for all other browsers. This is very possible - we already have IE specific stylesheets.

I'm not sure I understand what you mean. Which stylesheet needs to be modified on a per-browser basis to support a favicon link in the template?

None. I think what Don means (then again, I have no idea what he's talking about most of the time ... ) is that the IE-specific favicon could be included using conditional comments which we do already have in BP to include IE-specific stylesheets.

My personal opinion on this right now is "whatever", but that might be because I have other stuff on my mind right now However, we haven't seen any "complaints" about the current method so far (at least as far as I remember), so maybe it's really not that big of a deal. Then again, it's not really that much of a deal to implement. Oh, well.

YL

[Don Chambers]

I respect what Anson is proposing relative to BP. Should a diff be provided, I would be happy to review and commit as I am sure anyone else would do who has that ability.

I do know that the current BP code seems to work in every environment I have tested - which is certainly not every environment possible.

Whatever you guys want to do is fine with me.

[Anson]

Okay, I'll see what I can do this weekend.

[Anson]

Okay, attached is a diff for the following changes:

• Add a lastBuildDate element the RSS 2.0 template, which is always the current time (since comments aren't built periodically)
• Add a (commented-out) favicon link to Bulletproof's index.tpl in the standardized format used by browsers other than Internet Explorer.
• Add favicon links to the default admin template in both the standardized and IE formats, also commented out.
• Add a description block to the RSS 2.0 feed template with just the article's main body (excluding the extended body), escaped to avoid HTML. Change the values in the content:encoded block to to be unescaped CDATA of the body and extended body.

That last one is just an idea I had for how to best handle description vs. content:encoded, based on the best practices outlined here: http://www.rssboard.org/rss-profile#nam ... nt-encoded

I haven't tackled making the favicon a configurable setting yet.

[garvinhicking]

Hi!

Against which version did you create the patch? It won't apply to my SVN trunk:

Code: Select all

patching file rss.php
patching file index.tpl
Hunk #1 FAILED at 16.
1 out of 1 hunk FAILED -- saving rejects to file index.tpl.rej
patching file index.tpl
Hunk #1 FAILED at 5.
1 out of 1 hunk FAILED -- saving rejects to file index.tpl.rej
patching file feed_2.0.tpl
Hunk #1 FAILED at 16.
Hunk #2 FAILED at 38.
2 out of 2 hunks FAILED -- saving rejects to file feed_2.0.tpl.rej

I'm thinking maybe to create another version like "2.0plain" to offer RSS feeds in 2.0 format using description instead of content:encoded? That way we won't touch existing RSS feeds that at least I haven't heard many complaints about?!

Regards,
Garvin

[Anson]

Against which version did you create the patch?

I used 1.5.1, and I untarred it fresh from the version I downloaded to make sure I avoided any inadvertent edits from when I was upgrading 1.4.1->1.5.1. I can regenerate it against svn truck if you'd like, I wasn't expecting any of that to have changed between 1.5.1 and trunk. :-/

Just to double-check, I downloaded the diff from this board again, untarred the 1.5.1 tarball again, and ran it again, no problems.

Code: Select all

~$ tar zxvf serendipity-1.5.1.tar.gz
~$ cd serendipity
~/serendipity$ patch -p1 < ../s9y_1.5.diff
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff -ur serendipity/rss.php www/blog/rss.php
|--- serendipity/rss.php        Fri Jan 30 08:03:31 2009
|+++ www/blog/rss.php   Sat Jan  2 14:59:28 2010
--------------------------
Patching file rss.php using Plan A...
Hunk #1 succeeded at 135.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff -ur serendipity/templates/bulletproof/index.tpl www/blog/templates/bulletproof/index.tpl
|--- serendipity/templates/bulletproof/index.tpl        Thu Apr 23 09:08:09 2009
|+++ www/blog/templates/bulletproof/index.tpl   Sat Jan  2 17:27:35 2010
--------------------------
Patching file templates/bulletproof/index.tpl using Plan A...
Hunk #1 succeeded at 16.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff -ur serendipity/templates/default/admin/index.tpl www/blog/templates/default/admin/index.tpl
|--- serendipity/templates/default/admin/index.tpl      Fri May 22 03:20:51 2009
|+++ www/blog/templates/default/admin/index.tpl Sat Jan  2 15:04:06 2010
--------------------------
Patching file templates/default/admin/index.tpl using Plan A...
Hunk #1 succeeded at 5.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff -ur serendipity/templates/default/feed_2.0.tpl www/blog/templates/default/feed_2.0.tpl
|--- serendipity/templates/default/feed_2.0.tpl Tue Jul 25 04:42:28 2006
|+++ www/blog/templates/default/feed_2.0.tpl    Sun Jan 10 13:49:37 2010
--------------------------
Patching file templates/default/feed_2.0.tpl using Plan A...
Hunk #1 succeeded at 16.
Hunk #2 succeeded at 39.
done

I'm thinking maybe to create another version like "2.0plain" to offer RSS feeds in 2.0 format using description instead of content:encoded? That way we won't touch existing RSS feeds that at least I haven't heard many complaints about?!

I totally understand not wanting to fix what isn't broken. However, short of doing your own testing with the wide variety of RSS readers out there, it's hard to tell when and where this might or might not be broken, which is why I'm advocating following the recommendations of people who do test. Beyond that, though, my interests here are a- sharing my effort and b- reducing the effort required on my part for future upgrades. Your plan helps with both of those, so I'm fine with it in general.

[garvinhicking]

Hi!

That's strange, I also did not think that in SVN trunk there would be any changes. Would it be hard for you to patch against that? *smile*

Best regards,
Garvin

[Anson]

Hi!

That's strange, I also did not think that in SVN trunk there would be any changes. Would it be hard for you to patch against that? *smile*

Best regards,
Garvin

I am trying to find the time, but I may not be able to for a few weeks. I'll report back when I can.