Skip to content

fix problem with styles at the start of the server content#1768

Merged
1cg merged 1 commit intobigskysoftware:devfrom
dmgd:fix-history-nav-with-style
Sep 21, 2023
Merged

fix problem with styles at the start of the server content#1768
1cg merged 1 commit intobigskysoftware:devfrom
dmgd:fix-history-nav-with-style

Conversation

@dmgd
Copy link
Copy Markdown
Contributor

@dmgd dmgd commented Sep 6, 2023

I was playing around with htmx with a site that has styles embedded in weird places, and noticed that when I navigated through history, occasionally styles were disappearing.

After a bit of investigation I found makeFragment(..) and it all made (a bit more) sense. I've attached a screen recording showing the new tests before + after the fix. I recorded it with firefox but I checked it in chrome + safari too. Don't have IE to check that . . .

Screen.Recording.2023-09-06.at.23.37.50.mov

@dmgd dmgd changed the title fix problem with styles at the start of the server content when navigating through history fix problem with styles at the start of the server content Sep 13, 2023
@Telroshan Telroshan added the bug Something isn't working label Sep 19, 2023
@Telroshan
Copy link
Copy Markdown
Collaborator

Ran a quick test in the browser's console, confirming the issue:
image

The problem is that passing a <style> element to the DOMParser will have it in the head of the generated document fragment, while the parseHTML function retrieves its body, thus loses the style element in this case.
Wrapping it in a div makes it go in the body instead.

That's the same behaviour as for a script tag, which is already handled in the lib by this div wrapping, so seems like a good fix to me as it's reusing the same switch case

As for testing in IE @dmgd , you can run Edge in IE mode, see this PR for reference, in section How to test on IE11
For peace of mind, could you make sure it also works in IE mode ?

@dmgd
Copy link
Copy Markdown
Contributor Author

dmgd commented Sep 19, 2023

Thanks for the pointers on how to test against IE11. I've tested it with Edge in IE compatibility mode and it's working fine!

Screen.Recording.2023-09-20.at.00.34.04.mov

@Telroshan Telroshan added the ready for review Issues that are ready to be considered for merging label Sep 20, 2023
@Telroshan
Copy link
Copy Markdown
Collaborator

Perfect thanks!

@1cg 1cg merged commit 0ec900c into bigskysoftware:dev Sep 21, 2023
@dmgd dmgd deleted the fix-history-nav-with-style branch September 21, 2023 22:26
1cg pushed a commit that referenced this pull request Sep 22, 2023
fix problem with styles at the start of the server content when navigating through history
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready for review Issues that are ready to be considered for merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants