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

aklapper created this revision.Jul 13 2023, 13:21

Owners added a reviewer: O1: Blessed Committers.Jul 13 2023, 13:21

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptJul 13 2023, 13:21

aklapper requested review of this revision.Jul 13 2023, 13:21

Harbormaster completed remote builds in B667: Diff 1119.Jul 13 2023, 13:22

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.

This revision now requires changes to proceed.Jul 17 2023, 08:05

In D25346#10087, @avivey wrote:

I think this should be implemented using the getHead() method that's already defined.

Could you elaborate a bit, please? I'm not sure what exactly not to do here, and what to do instead. Thanks in advance!

In D25346#10708, @aklapper wrote:

In D25346#10087, @avivey wrote:

I think this should be implemented using the getHead() method that's already defined.

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.