accept empty xml space for content:encoded #211
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@JLugagne Can you show a quick example of this? I think I understand, but want to be completely sure. It sounds like this might be better to be fixed in goxpp, but I'd like to make sure I grok what the issue is. |
JLugagne
force-pushed
the
accept_empty_space
branch
2 times, most recently
from
9bb0d94 to
30e565b
Compare
JLugagne
force-pushed
the
accept_empty_space
branch
from
30e565b to
a9dcefd
Compare
|
Sure, one of the RSS feed I had to use was having xmlns:content in the root tag but in the content:enconded tag it was defining only the xmlns attribute (you can check the unit test I have added). In that case I guess that it means xmlns refer to the namespace of the tag? |
cristoper
added a commit
to cristoper/gofeed
that referenced
this pull request
Feb 29, 2024
PR mmcdole#220 introduced a failing test for detecting images in the "content" element. It should instead be testing the "content:encoded" element. But that uncovered an issue with how extensions were being detected (the "content" namespace was being detected as an extension namespace). As a more robust way of checking for the "content" namespace, this PR exposes `shared.PrefixForNamspace()` as a public function so it can be used in the rss parser. This provides a solution to PR mmcdole#211 (and includes @JLugagne's test case from that PR). Once the fixes to xml:base handling in mmcdole#222 are merged, this should fix the remaining failing test reported in mmcdole#210.
cristoper
added a commit
to cristoper/gofeed
that referenced
this pull request
Feb 29, 2024
PR mmcdole#220 introduced a failing test for detecting images in the "content" element. It should instead be testing the "content:encoded" element. But that uncovered an issue with how extensions were being detected (the "content" namespace was being detected as an extension namespace). As a more robust way of checking for the "content" namespace, this PR exposes `shared.PrefixForNamspace()` as a public function so it can be used in the rss parser. This provides a solution to PR mmcdole#211 (and includes a test based on @JLugagne's test case from that PR). Once the fixes to xml:base handling in mmcdole#222 are merged, this should fix the remaining failing test reported in mmcdole#210.
cristoper
added a commit
to cristoper/gofeed
that referenced
this pull request
Feb 29, 2024
PR mmcdole#220 introduced a failing test for detecting images in the "content" element. It should instead be testing the "content:encoded" element. But that uncovered an issue with how extensions were being detected (the "content" namespace was being detected as an extension namespace). As a more robust way of checking for the "content" namespace, this PR exposes `shared.PrefixForNamspace()` as a public function so it can be used in the rss parser. This should also fix PR mmcdole#211 (and includes @JLugagne's test case from that PR). Once the fixes to xml:base handling in mmcdole#222 are merged, this should fix the remaining failing test reported in mmcdole#210.
mmcdole
pushed a commit
that referenced
this pull request
Mar 1, 2024
PR #220 introduced a failing test for detecting images in the "content" element. It should instead be testing the "content:encoded" element. But that uncovered an issue with how extensions were being detected (the "content" namespace was being detected as an extension namespace). As a more robust way of checking for the "content" namespace, this PR exposes `shared.PrefixForNamspace()` as a public function so it can be used in the rss parser. This should also fix PR #211 (and includes @JLugagne's test case from that PR). Once the fixes to xml:base handling in #222 are merged, this should fix the remaining failing test reported in #210.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
in some cases, the space is empty to use the tag namespace as default. To manage this I have added an OR conditions in rss/parser.go.
Alternative: improve goxpp to use tag namespace as xmlns default if empty ?