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

[5.2] Fix @parent keyword escaping #11558

Closed
wants to merge 1 commit into from
Closed

[5.2] Fix @parent keyword escaping #11558

wants to merge 1 commit into from

Conversation

Ruby184
Copy link
Contributor

@Ruby184 Ruby184 commented Dec 27, 2015

This PR is fix for issue #10068 and it's trying to properly handle @parent keywords in Blade sections and ensure proper section extending. The solution is hardly based on previously reverted commit #10122 with a few tweaks.

TODO list:

  • Fix original issue [5.1] [5.3] @parent tag is not escaping #10068.
  • Add test to ensure @parent keyword is properly handled.
  • Edit test for multiple section extending to ensure we will not introduce bugs like @parent totally breaks since 5.1.13 #10156.
  • Fix @append behaviour to properly handle parent marks.
  • Add tests for section appending.
  • Confirmation from @abhimanyusharma003 @parent keyword is properly escaped.
  • Confirmation form @atmediauk this fix is working as expected and has no side effects for him.

Thank you guys. Special thanks to @mspiderv for helping me with figuring out how to fix it and @dmgawel for previous try to fix this issue.

@Ruby184 Ruby184 changed the title [5.2] Fix @parent keyword escaping [5.2] Fix @parent keyword escaping Dec 27, 2015
@GrahamCampbell
Copy link
Member

I think this should go to 5.3, not 5.2.

@GrahamCampbell
Copy link
Member

Infact, please can you do this, because it requires views to be re-compiled, so upgrading isn't as easy as just running composer update, you need to clear the compiled views, right.

@GrahamCampbell
Copy link
Member

Also, if this were a bug fix with no upgrading issues, then this would have to be targeted at L5.1 anyway btw. ;)

@GrahamCampbell
Copy link
Member

Thanks for the PR though. If you could be so kind as to continue this on 5.3, that would be awesome. ;)

@abhimanyu003
Copy link
Contributor

This is working perfectly fine for me. I'm not sure why this will be included in 5.3, issue is still in 5.1 and 5.2 ?

@GrahamCampbell
Copy link
Member

It only works if you clear the previously cached views.

@GrahamCampbell
Copy link
Member

If it was a worry free bug fix, it would go to 5.1, otherwise, because of it's breaking nature, it must go to 5.3. There's no scenario in which it would go to 5.2.

@Ruby184
Copy link
Contributor Author

Ruby184 commented Dec 27, 2015

Yes you are right @GrahamCampbell, this is a breaking change and composer update without clearing cached views leads to broken view sections without expansion of @parent keywords.
Thanks for the review and explanation. I will send another PR, which will target 5.3 branch.
And thanks @abhimanyusharma003 for testing this fix.

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.

3 participants