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

[Bug] [1.8.01]: CSS minification breaks CSS and also minification cannot be disabled #565

Open
kindaro opened this issue Jan 4, 2025 · 6 comments
Labels
bug Something isn't working has-link This issue provides a link (possibly to a resource helpful to reproduce the issue)

Comments

@kindaro
Copy link

kindaro commented Jan 4, 2025

What happened?

Firstly let me say that this project is unbelievably well done. It is delightful to work with. The code is easy to navigate.

I have some fairly elaborate CSS snippets in my vault. I rejoiced when I saw that this plug-in takes care to export all of my enabled snippets carefully along with my notes.

However, it turns out that CSS snippets are not exported correctly. Specifically, nesting does not work — only the top level information is kept, and all the nested styles are dropped.

I know specifically that minification is to blame, because I commented out the relevant code in minifyAsset in the file scripts/html-generation/assets/asset.ts and it solved the problem. I checked this on the commit tagged 1.8.01, which is the version generally available for installation.

There is nothing we can do short of replacing the minifier library. I figure they were not keeping up to date with browser technology, and their parser does not handle nesting in CSS. Until then, the best idea is to switch minification off.

However, it turns out that setting "minifyHTML": false in data.json does nothing. (I checked and this is the flag that can be set in HTML export settings, section Export Options, Minify HTML.) Everything is still minified and CSS is still broken.

I notice that this setting disappeared in the β version. I do not know why it was removed. Maybe we should bring it back to safeguard against such issues in the future.

Reproduction

Create a new vault. Install the plug-in. Add the following custom snippet to the vault:

p {
  background: green;
}

ul {
  li {
    background: red;
  }
}

Enable this snippet.

Create the following note:

paragraph

- list item

Observe that the paragraph has green background and the list item has red background. Export the note. Open the note in a web browser. Observe that the paragraph still has green background but the list item no longer has red background.

Last Working Version

No response

Version

1.8.01, latest β, latest commit e910c95

Operating System

Linux

Obsidian Version

1.7.7

Which browsers can you reproduce this in?

Firefox

Relevant log output

Log:
[WARNING] 	Creating an RSS feed requires a site url to be set in the export settings.
[WARNING] 	No existing export metadata found. All files will be exported.
[WARNING] 	No existing search index found. All files will be exported.
[WARNING] 	No existing RSS feed found. All files will be exported.
[INFO] 	Closing render window


Settings:
	settingsVersion --------- 1.9.0
	makeOfflineCompatible     false
	inlineAssets ------------ false
	includePluginCSS          1 plugins included
	includeSvelteCSS -------- true
	titleProperty             title
	customHeadContentPath ---
	faviconPath              
	documentWidth ----------- 40em
	sidebarWidth              20em
	minOutlineCollapse ------ 2
	startOutlineCollapsed     false
	allowFoldingHeadings ---- true
	allowFoldingLists         true
	allowResizingSidebars --- true
	logLevel                  warning
	minifyHTML -------------- true
	makeNamesWebStyle         true
	onlyExportModified ------ true
	deleteOldFiles            true
	addThemeToggle ---------- true
	addOutline                true
	addFileNav -------------- true
	addSearchBar              true
	addGraphView ------------ true
	addTitle                  true
	addRSSFeed -------------- true
	siteURL                  
	authorName --------------
	vaultTitle                example-1
	exportPreset ------------ website
	openAfterExport           false
	graphAttractionForce ---- 1
	graphLinkLength           10
	graphRepulsionForce ----- 150
	graphCentralForce         3
	graphEdgePruning -------- 100
	graphMinNodeSize          3
	graphMaxNodeSize -------- 7
	showDefaultTreeIcons      false
	emojiStyle -------------- Native
	defaultFileIcon           lucide//file
	defaultFolderIcon ------- lucide//folder
	defaultMediaIcon          lucide//file-image
	exportPath -------------- /tmp/z6
	filesToExport             0


Enabled Plugins:
	Webpage HTML Export

Additional Info

No response

@kindaro kindaro added the bug Something isn't working label Jan 4, 2025
@github-actions github-actions bot added the has-link This issue provides a link (possibly to a resource helpful to reproduce the issue) label Jan 4, 2025
@github-actions github-actions bot changed the title [Bug]: CSS minification breaks CSS and also minification cannot be disabled [Bug] [1.8.01]: CSS minification breaks CSS and also minification cannot be disabled Jan 4, 2025
kindaro added a commit to kindaro/obsidian-webpage-export that referenced this issue Jan 4, 2025
@kindaro
Copy link
Author

kindaro commented Jan 4, 2025

This is my patch for the time being:

diff --git a/scripts/html-generation/assets/snippet-styles.ts b/scripts/html-generation/assets/snippet-styles.ts
index 8a3ee0c..1a05e7c 100644
--- a/scripts/html-generation/assets/snippet-styles.ts
+++ b/scripts/html-generation/assets/snippet-styles.ts
@@ -9,7 +9,7 @@ export class SnippetStyles extends Asset

     constructor()
     {
-        super("snippets.css", "", AssetType.Style, InlinePolicy.AutoHead, true, Mutability.Dynamic, LoadMethod.Async, 2);
+        super("snippets.css", "", AssetType.Style, InlinePolicy.AutoHead, false, Mutability.Dynamic, LoadMethod.Async, 2);
     }

     private static getEnabledSnippets(): string[]

@KosmosisDire
Copy link
Owner

KosmosisDire commented Jan 9, 2025

Thanks, to be clear is this still broken in the latest beta version?

I am not sure if there is a clean solution for this, because I don't want the solution to this to be that the user has to turn on minification for everything. But also having a bunch of toggle for all the different thing that could be minified seems a bit too much. Maybe there is a way to ensure that this case is not broken by minification.

@KosmosisDire
Copy link
Owner

KosmosisDire commented Jan 9, 2025

The minifyHTML was removed because it wasn't being used, and it is a very poor choice to not minify EVERYTHING. Maybe some things would be okay. I agree that replacing the minification library could work too. I guess that would be better than trying to work around it.

Maybe there is a library that can de-nest the css too if there isn't a good minificiation library.

@kindaro
Copy link
Author

kindaro commented Jan 13, 2025

Thanks, to be clear is this still broken in the latest beta version?

To the best of my knowledge, it is still broken in the latest beta version.

@kindaro
Copy link
Author

kindaro commented Jan 13, 2025

For me, being able to switch minification off would be valuable, because, if I want to publish generated code on my web site, I should like first to review it, and minification makes reviewing generated code much harder.

@KosmosisDire
Copy link
Owner

@kindaro This makes sense as a use case. I will consider adding a toggle back in for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working has-link This issue provides a link (possibly to a resource helpful to reproduce the issue)
Projects
Development

No branches or pull requests

2 participants