HTML entities double-escaped in titles

Yeeeeaaaah... you didn't see this then?

It works, and it only changes one line of code...

No, didn't notice it. Thank you. (I think I was confused by the "rendering" bit; or perhaps if you mentioned HTML entities in the title?)

It does only change one line, while mine changes 8 or so, if I recall. But I think what may be more important is making sure it doesn't break anything else. Yours gets the set_value function to return the right value, but you'd have to fix the return value for all externally facing functions as well, especially the nodeapi hook. But more importantly, I think titles should probably be handled the same in this module as they are in native Drupal without the module. The t(@text) handling of this would have worked fine in 4.6 and 4.7, but in 5 for these to internally be handled the same, that has to be changed to t(!text). That is what my code does, and then I don't have to "kludge" any of the return values at the end.

Are you saying that this is a porting error then...?

I didn't (at the time) spot anywhere else in the module that was exposing the title variable, so I thought it was fairly safe to ""kludge"" it :)
If your patch is more appropriate to D5 coding standards, then I'll tag mine as duff and jump on board this! :)

Well, the nodeapi hook uses the title var as well, but more to the point, the title var is the same as is used in Drupal core, and I don't know whether there are any places where that is used in core that this module doesn't override, and I would not want to take that risk.

And yes, this is a very common porting bug. I've run into similar bugs with other modules as well. 4.7 didn't automatically check_plain results of t() functions, while 5 does. I do feel this is the right way to address this bug; then again, I wrote the patch, so I may be biased :-) which is why I left it up for review. I'd love to hear anyone else's opinions on it.

FYI, I have unescaped page titles in other areas as well, that are more likely to be associated with problems in core than anything else (I think) - I renamed a blog "News & Events" and the ampersand is coming thru as you-know-what.

Thanks ! Works for me, 's and &'s comes out ok.

I tried to solve this and the following problem earlier (and was partially successful - failed on the 's). Even with the above patch, if you use Views module, you'll see the <em> tags in the html title in edit mode. I don't know if this appears elsewhere in other contexts, but the following (adapted from my earlier solution) resolved it :

Around page_title.module:158, apply the strip_tags for the $drupal_title variable. So

becomes:

This solution is based on looking at /themes/engine/phptemplate.engine. There's a strip_tags around drupal_get_title() in constructing $head_title, whereas none was in the current module.

Does anyone think this is worth commiting ?

gnassar's patch and freeman's suggestions committed...

http://drupal.org/cvs?commit=65459

Thanks guys

Exactly the same behaviour in 5.x-2.2 version of the module. Apostrophes are put out as '.
Used an Apostrophe for user's blogs: [page-title] | [author-name-raw]'s Blog.

Yep, I'm still experiencing this on 5.x-2.2.

Removing the call to check_plain() in the return statement of page_title_page_get_title() solves the issue. But I'm not sure if this user input is sanitized elsewhere, and thus I'm not sure if this is safe to do.

I've submitted some fixes for this to the dev branches - can you guys please review and let me know if it fixes your problems? I cannot replicate any of the old double-encode situations on my dev box...

Common problems (for me) were:
(1) Node Titles with ampersands and/or greater than/less than symbols or quotes
(2) Nodes with Page Titles that contain characters mentioned in (1)
(3) Menu Titles containing encodable characters such as ampersands or quotes, such as admin/logs/access-denied...