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

rel=tag hentry/hreview backcompat #164

Merged
merged 6 commits into from
Mar 26, 2018
Merged

Conversation

gRegorLove
Copy link
Member

Fixes #157

@gRegorLove gRegorLove requested a review from aaronpk March 24, 2018 21:37
@Zegnat
Copy link
Member

Zegnat commented Mar 25, 2018

I still have to read up on how php-mf2 actually handles backwards compatibility parsing but this looks like a good way to rewrite the rel links.

One thing that comes to mind – but may already apply to other back-compat work – is what happens if the link is inside an element that will be parsed as an e-* property? Sounds to me like the parser will need to exclude these newly added <data> elements from its output there?

Copy link
Member

@aaronpk aaronpk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zegnat makes a good point. Currently this ends up inserting the data element into the content HTML and text. I've added a failing test and sent it as a PR against your branch. If you merge gRegorLove#2 then this PR will be updated with the failing test.

gRegorLove and others added 2 commits March 25, 2018 12:08
add failing test for excluding the generated data element in rel tag backcompat
Fixed test to include the original rel=tag HTML/text
@gRegorLove gRegorLove added this to the 0.4.2 milestone Mar 25, 2018
@Zegnat
Copy link
Member

Zegnat commented Mar 26, 2018

If the test can be rerun and it turns out it passes I say lets merge this 👍

One downside discussed on IRC seems to be fringe and not worth fretting over for now. Maybe?

Yeah, it works, but still fails if there's an hentry.entry-content nested in an hentry.entry-content.

entry-content in entry-content is odd for sures, probably safe to ignore for now

@aaronpk aaronpk merged commit 347d301 into microformats:master Mar 26, 2018
@gRegorLove gRegorLove deleted the issue157 branch July 8, 2022 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants