Make live reload script injection more reliable.#746
Open
DominoPivot wants to merge 1 commit intomicrosoft:mainfrom
Open
Make live reload script injection more reliable.#746DominoPivot wants to merge 1 commit intomicrosoft:mainfrom
DominoPivot wants to merge 1 commit intomicrosoft:mainfrom
Conversation
Author
|
@microsoft-github-policy-service agree |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR replaces the utility function which injects a live reload script in HTML files, addressing issues mentioned in #711 such as:
<head >before falling back to searching for the<body >, and again before falling back to searching for<!DOCTYPE >. This has a small performance cost which could become worse as we make the search itself more robust.<head>is omitted, the<header>tag can be mistaken for it, possibly affecting page layout. This can be solved by changing the regular expression so that the tag name must be followed by>or whitespace.<script>and<title>can be mistaken for the<head>and<body>tags. This can cause the live reload to stop working, or break parsing for the rest of the page.The solution I propose is to find the start of the head by scanning from the start of the file and explicitly consuming the DOCTYPE, html start tag and head start tag, along with any whitespace and comments before and between them. The script is injected before any other element or body text.
To follow the spirit of the existing code, I fall back to consuming the body start tag if the head is missing, but in practice all HTML files should have a title in which case the script will be injected before it.
This doesn't solve issues where the
>character appears in a quoted attribute value on the html or head start tag. Do tell if you think this case should be handled properly.⚠ I was unable to run and therefore write my own tests for this, I get an error when
runTeststries to download VS code. Can someone familiar with this repository help?⚠ It would probably be a good idea to add a .gitattributes file to force CRLF line endings, that tripped me up since my global git config does the opposite.