Details
- Reviewers
avivey - Group Reviewers
O1: Blessed Committers - Maniphest Tasks
- T15550: Add HTML header for Atom/RSS feed discovery on Phame blog pages
- Commits
- rP39b576b145ae: Add HTML header for Atom/RSS discovery on Phame blog pages
Go to /phame/blog/view/1/ and check the HTML source code. See an additional <link> item with type="application/atom+xml" in the <head> section.
Diff Detail
- Repository
- rP Phorge
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
I think this should be implemented using the getHead() method that's already defined.
| src/view/page/PhabricatorStandardPageView.php | ||
|---|---|---|
| 385–386 | You can have PHP do the type checking, but I'm not sure that this is needed - phutil_implode_html already accepts unsafe strings and converts them to safe html. | |
Could you elaborate a bit, please? I'm not sure what exactly not to do here, and what to do instead. Thanks in advance!
I was thinking about the getHead() that's being called in line 411, but now I see that it's already being overriden in PhabricatorStandardPageView.
I'm a little weary of adding addHeadItem() method to Page, because it opens up the space of places strange things can enter the head; I'd rather have a dedicated PageWithAlternateLink class (or mixin?) that will explicitly override getHead and add it, but that's not really possible either.
| src/view/page/PhabricatorStandardPageView.php | ||
|---|---|---|
| 385–386 | Probably should bother - head items are not normal content, and can't ever be something that isn't explicitly crafted to work in a head element. | |