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

@import local files for convenience #14

Closed
folknor opened this issue Dec 11, 2017 · 11 comments
Closed

@import local files for convenience #14

folknor opened this issue Dec 11, 2017 · 11 comments

Comments

@folknor
Copy link

folknor commented Dec 11, 2017

Hi!

So I already had both custom userContent.css and userChrome.css, but of course I have to use yours, they are so fantastic.

Which means I merged yours with my local ones, but it's a pain to update.

Can you possibly add something equivalent to

@import "localContent.css" and @import "localChrome.css" at the bottom of your CSS files? Then I could just git pull and put my own selectors in those files, and never have any conflicts.

Firefox will stop parsing at those statements if the files do not exist, so they must be at the bottom.

Thank you for the work you've done, it looks amazing.

@folknor
Copy link
Author

folknor commented Dec 11, 2017

I realise that this might have unintended consequences that we are unable to foresee.
And also that there might be some better way of doing it that I haven't considered.

@folknor
Copy link
Author

folknor commented Dec 11, 2017

I didn't think this through at all - @import statements obviously have to be at the top of the file.
I filed this request before I did any investigation into the issue, which was a mistake.

Hopefully we can think about the issue and come up with some solution.

@folknor
Copy link
Author

folknor commented Dec 11, 2017

Alright, what I did instead is:

  1. git clone this to some folder
  2. cd ~/.mozilla/firefox/0xdeadbeef/chrome
  3. ln -s /home/user/git/firefox-gnome-theme/ theme

Then I put this in userChrome.css above my content:
@import "theme/ui/theme-light.css";

And this in userContent.css above my content:
@import "theme/userContent.css";

So I'll close this issue now. Apologies for the spam!

@folknor folknor closed this as completed Dec 11, 2017
@lunakurame
Copy link
Owner

Hi!
Thanks for your kind words. Actually I wanted to add that feature for quite a while, because I do have my own custom styles too. I haven't updated my own setup for like a week because I'm too lazy to merge them in. Unfortunately @import doesn't work in the userContent.css file due a bug in Electrolysis (Firefox' multi-process rendering engine): https://bugzilla.mozilla.org/show_bug.cgi?id=1416184. Or does it work for you? Did you test your solution before closing this issue? It should work in the userChrome.css file, but not in userContent.css as far as I know, unless you turn off Electrolysis, but that would give you a performance penalty.

I'm thinking about a different way to allow importing custom styles, and it would be nice to also preserve the theme config (currently there are a few things you can enable in userChrome.css and there will be more, but all your config is lost every time you git pull). I'm not sure yet how to do that without overcomplicating things, but I think a Bash script would be good enough. The config would be generated the first time you install the theme (running something like install.sh and answering a few questions) and then when you wanna update, you just run something like

git pull
./update.sh

Any changes in your custom styles for the userContent.css file won't work until you ./update.sh again, because of that Electrolysis bug, but still it's just a single command you need to run after any changes. What do you think? Or maybe you have some suggestions? I'd like to make it work properly once and for good to not scare other users with constantly changing installation instructions. 😛

@lunakurame lunakurame reopened this Dec 11, 2017
@folknor
Copy link
Author

folknor commented Dec 11, 2017

Ah, yes, I have e10s disabled, and have apparently for a long time. I always run firefox nightly built from source, and have done that for probably over 10 years on linux. Which means that I sometimes have to explicitly disable new features that keep crashing - browser.tabs.remote.autostart.2 set to false is apparently one of them.

But yes, @import does work for me with e10s disabled. That is to say, it works with that pref set to false. Whether or not that enables or disables e10s, I can not say, except that I trust the comment on BMO at face value and can't be bothered to check it further.

My thoughts are that personally I don't care about Windows-users, so a script would be fine - but also that it's working for me now and I've apparently had e10s disabled since its inception - and performance has never been a problem for me, so I am certainly willing to wait until the BMO issue is resolved, and continue with the way I solved it in my previous comment.

But also that it seems more likely that the userContent.css filename is explicitly sandboxed for access from the content processs than it being related to e10s. I can do some code lookup on DXR a bit later and check it out!

@folknor
Copy link
Author

folknor commented Dec 11, 2017

Hm, so, there's no doubt that I have e10s enabled now after resetting some preferences (about:support says "Multiprocess Windows 2/2 (Enabled by default)"), and @import from userContent.css is definitely working for me.

https://dxr.mozilla.org/mozilla-central/source/security/sandbox/linux/broker/SandboxBrokerPolicyFactory.cpp#466

This is the first rule of my userContent.css:

@import "theme/userContent.css";

@namespace html "http://www.w3.org/1999/xhtml";
@namespace xul "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";

@-moz-document domain(reddit.com) {
	section.infobar:nth-child(1) {
		display: none !important;
	}
}

And I can comment out/in the top line to enable/disable your userContent changes.

@lunakurame
Copy link
Owner

Well this is weird. Something just randomly started working for me. I've got two keys in my about:config:

  • browser.tabs.remote.autostart
  • browser.tabs.remote.autostart.2

By default the first one is set to false and the second one to true. If I enable both, nothing changes. If I disable the second one, about:support shows "Multiprocess Windows" is disabled. When I enable the first one, but disable the second one, "Multiprocess Windows" is enabled, but "Web Content Processes" is set to just 1 instead of usual 4. Also doesn't matter which settings I pick, @import always works for XUL pages (about:addons, about:preferences), but if the second option is enabled (which is the default), then @import doesn't work for HTML pages (like about:newtab). HTML pages only work properly when the second option is disabled, which forces "Web Content Processes" to be 1. So as I understand it, XUL pages work just fine, but if you want HTML pages to work properly, you can still keep multiprocess windows, but you can't keep multiprocess web content. And Firefox uses both XUL and HTML for its internal pages.

@folknor
Copy link
Author

folknor commented Dec 12, 2017

browser.tabs.remote.autostart: true
browser.tabs.remote.autostart.2: false
Multiprocess Windows 	1/1 (Enabled by default)
Web Content Processes 	4/4

browser.tabs.remote.autostart: true
browser.tabs.remote.autostart.2: true
Multiprocess Windows 	1/1 (Enabled by default)
Web Content Processes 	4/4

browser.tabs.remote.autostart: false
browser.tabs.remote.autostart.2: true
Multiprocess Windows 	0/1 (Disabled)
Web Content Processes 	no-show

browser.tabs.remote.autostart: false
browser.tabs.remote.autostart.2: false
Multiprocess Windows 	0/1 (Disabled)
Web Content Processes 	no-show

As far as I can see, browser.tabs.remote.autostart.2 has no impact at all, which is backed up by the lack of any result from DXR on it: https://dxr.mozilla.org/mozilla-central/search?q=browser.tabs.remote.autostart&redirect=false

@folknor
Copy link
Author

folknor commented Dec 12, 2017

https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/Troubleshoot.jsm#212 explains what "Web Content Processes" means:

    // Services.ppmm.childCount is a count of how many processes currently
    // exist that might respond to messages sent through the ppmm, including
    // the parent process. So we subtract the parent process with the "- 1",
    // and that’s how many content processes we’re waiting for.
    data.currentContentProcesses = Services.ppmm.childCount - 1;
    data.maxContentProcesses = Services.appinfo.maxWebProcessCount;

ppmm implements, according to https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Services.jsm "nsIMessageBroadcaster" and "nsIProcessScriptLoader"

https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIMessageBroadcaster#childCount

https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Message_Manager/Message_manager_overview#Global_parent_process_message_manager

@lunakurame
Copy link
Owner

lunakurame commented Dec 15, 2017

I see you are talking about Firefox Nightly again. I just checked and it does work as you describe it. However, Firefox 57, which is the current stable release, still has this problem: https://dxr.mozilla.org/mozilla-release/search?q=browser.tabs.remote.autostart. Firefox 58.0b5 appears to behave like FF Nightly, so at least I'm glad it's gonna be fixed as soon as the new release rolls out.

Anyway, I think we could temporary sacrifice that option to make @import work properly in the userContent.css file, since the problem will disappear on January 23 (FF58 release date). It was nice to talk to someone who knows Firefox' code better than I do. Thanks, I'll commit a patch soon.

lunakurame added a commit that referenced this issue Dec 15, 2017
- separate variants for different GNOME versions, issue #3
- prepare for importing a custom stylesheet, issue #14
- everything disabled by default to allow future custom styles to preserve
  config between updates, issue #14
lunakurame added a commit that referenced this issue Dec 15, 2017
- separate colors from the actual rules, issue #2
- prepare for importing a custom stylesheet, issue #14
- everything disabled by default to allow future custom styles to preserve
  config between updates, issue #14
@lunakurame
Copy link
Owner

Okay after trying to implement this, I noticed @import doesn't work for HTML content in Firefox 57 if any of those are set to true:

  • browser.tabs.remote.autostart
  • browser.tabs.remote.autostart.2

I don't know why it worked before with browser.tabs.remote.autostart enabled, maybe they implemented a RNG... Anyway, I give up with that bug, either you disable e10s or you can't use userContent.css for HTML content. This applies only for Firefox 57 tho.

I just added @imports for customChrome.css and customContent.css, you can use them for your own styles. Also since all options are now disabled by default, you can store your config in these too and it's gonna survive when you git pull (copy and paste the relevant @imports... unless you prefer setting them up manually every time).

And again, thanks for help.

@lunakurame lunakurame added this to the Firefox 57 milestone Dec 16, 2017
andrew-ls pushed a commit to andrew-ls/firefox-gnome-theme that referenced this issue Oct 15, 2019
andrew-ls pushed a commit to andrew-ls/firefox-gnome-theme that referenced this issue Oct 15, 2019
andrew-ls pushed a commit to andrew-ls/firefox-gnome-theme that referenced this issue Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants