Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-enable xml:base for all supported RSS formats #723

Merged
merged 1 commit into from
Feb 28, 2022

Conversation

Alkarex
Copy link
Contributor

@Alkarex Alkarex commented Feb 27, 2022

SimplePie already has the mechanism for supporting xml:base but it is disabled for some feed types.

So this PR reverts the commit disabling it e49c578 (see also my question there)

Specification, e.g. Atom:

Any element defined by this specification MAY have an xml:base attribute

While RSS 2.0 is silent on the matter https://cyber.harvard.edu/rss/relativeURI.html , here are examples of use from the same epoch:

Reverts simplepie@e49c578

Specification, e.g. Atom:
* https://datatracker.ietf.org/doc/html/rfc4287#section-2

> Any element defined by this specification MAY have an xml:base attribute

While RSS 2.0 is silent on the matter https://cyber.harvard.edu/rss/relativeURI.html , here are examples of use from the same epoch:
* https://rssweblog.com/?guid=20050627094747
* https://www.drupal.org/project/views/issues/943762
Alkarex referenced this pull request in FreshRSS/FreshRSS Feb 27, 2022
And fix/revert `xml:base` support in SimplePie simplepie/simplepie@e49c578
@gsnedders
Copy link
Contributor

To respond to @Alkarex's comment on e49c578#commitcomment-67585150:

I know this is 15 years old already, but if @gsnedders is still around and remembers the rationale for disabling xml:base for RSS formats, I am interested :-)

I have absolutely no idea. 🙃

Quite possibly trying to avoid doing anything complex or treating RSS is ways more sophisticated than other implementations did?

Clearly we should time travel and teach 15-year-old me how to write better commit messages. 😅

@mblaney
Copy link
Member

mblaney commented Feb 28, 2022

thanks @Alkarex seems ok if the tests still pass :-)

@mblaney mblaney merged commit 3e2fa52 into simplepie:master Feb 28, 2022
@Alkarex Alkarex deleted the xml_base-for-RSS branch February 28, 2022 00:35
Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Feb 28, 2022
Alkarex added a commit to FreshRSS/FreshRSS that referenced this pull request Feb 28, 2022
* More PHP type hints for Fever
Follow-up of #4201
Related to #4200

* Detail

* Draft

* Progress

* More draft

* Fix thumbnail PHP type hint
#4215

* More types

* A bit more

* Refactor FreshRSS_Entry::fromArray

* Progress

* Starts to work

* Categories

* Fonctional

* Layout update

* Fix relative URLs

* Cache system

* Forgotten files

* Remove a debug line

* Automatic form validation of XPath expressions

* data-leave-validation

* Fix reload action

* Simpler examples

* Fix column type for PostgreSQL

* Enforce HTTP encoding

* Readme

* Fix get full content

* target="_blank"

* gitignore

* htmlspecialchars_utf8

* Implement HTML <base>
And fix/revert `xml:base` support in SimplePie simplepie/simplepie@e49c578

* SimplePie upstream PR merged
simplepie/simplepie#723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants