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

Inject code to last candidate #2705

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

twhlynch
Copy link

@twhlynch twhlynch commented Feb 26, 2023

Rather than injecting into the first match of "</body>" , it will inject into the last found occurrence of it. This will avoid overwriting code following the tag. Also works for the head regex.

PR Type

What kind of change does this PR introduce?

Fix injection candidate regex

[x] Bugfix
[ ] Feature
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other: <!-- Please describe: -->

What is the current behavior?

matches the first occurrence of a tag and overrides code

Issue Number: N/A (there were multiple issues referring to this issue)

What is the new behavior?

matches the last occurrence of a tag

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Rather than injecting into the first match of "</body>" , it will inject into the last found occurrence of it. This will avoid overwriting code following the tag.
Also works for the head regex.
@twhlynch
Copy link
Author

adjusting the head regex may not fix the issue, but the body fix will, and it is the most commonly used / reported issue

@williamthome
Copy link

I'm facing an issue due to this. Is there any plan to merge this fix?
For example:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
</head>
<body>
    <script>
        "</body>"
    </script>
</body>
</html>

Results in:
image

@twhlynch
Copy link
Author

twhlynch commented Sep 17, 2023

I'm facing an issue due to this. Is there any plan to merge this fix?

Highly doubt it. The extension hasn't been updated for over a year.

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.

None yet

2 participants