Skip to content

Core: detect incorrectly nested elements #338

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Core: detect incorrectly nested elements #338

wants to merge 2 commits into from

Conversation

dmethvin
Copy link
Member

@dmethvin dmethvin commented Apr 11, 2020 •
edited
Loading

Basic approach: If our old jQuery.htmlPrefilter would have changed the string and the resulting actual HTML string is different, give a warning.

dmethvin
dmethvin commented Apr 11, 2020
View reviewed changes
@dmethvin
Copy link
Member Author

@mgol This seems to work pretty well. Thoughts?

mgol
mgol reviewed Apr 11, 2020
View reviewed changes
@mgol mgol mentioned this pull request Apr 11, 2020
Closed
@dmethvin
Copy link
Member Author

Using .htmlPrefilter() seems like a good idea, I'll switch to that and add a few more tests.

@dmethvin dmethvin force-pushed the warn-on-prefilter branch from cdb1eeb to e0a1e0e Compare April 12, 2020 01:25
dmethvin
dmethvin commented Apr 13, 2020
View reviewed changes
dmethvin
dmethvin commented Apr 13, 2020
View reviewed changes
dmethvin
dmethvin commented Apr 13, 2020
View reviewed changes
@dmethvin
Copy link
Member Author

@mgol Take a look when you get a chance and give me your thoughts.

mgol
mgol requested changes Apr 14, 2020
View reviewed changes
Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

It's coming up pretty nicely!

I left some comments.

@mgol mgol mentioned this pull request Apr 30, 2020
Closed
@dmethvin dmethvin force-pushed the warn-on-prefilter branch 4 times, most recently from c61ae8f to 446cd99 Compare May 2, 2020 20:19
@dmethvin dmethvin marked this pull request as ready for review May 2, 2020 20:27
@dmethvin dmethvin requested a review from mgol May 2, 2020 20:32
dmethvin
dmethvin commented May 2, 2020
View reviewed changes
mgol
mgol requested changes May 2, 2020
View reviewed changes
Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

I added two comments, one of which is quite important, otherwise looks good to me!

@dmethvin dmethvin force-pushed the warn-on-prefilter branch from d02987d to ade45fa Compare May 2, 2020 22:19
mgol
mgol approved these changes May 2, 2020
View reviewed changes
Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

Looks good! Just one non-blocking suggestion.

@mgol mgol mentioned this pull request May 3, 2020
Merged
mgol
mgol approved these changes May 3, 2020
View reviewed changes
Copy link
Member

@mgol mgol left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Everything looks good now! Let's get this landed so that I can rebase & merge my Rollup PR. :)

@dmethvin dmethvin added this to the 3.3.0 milestone May 3, 2020
@dmethvin dmethvin closed this in 5241ccd May 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgol mgol mgol approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@dmethvin @mgol