search

lunakurame / firefox-gnome-theme

A theme for Firefox 57+ matching GNOME Adwaita.
The Unlicense
200 stars 15 forks source link

Complete 3.26 light/dark themes, and various improvements #21

Closed smithfred closed 6 years ago

smithfred commented 6 years ago

https://github.com/smithfred/firefox-gnome-theme-3.26-layer

It has working/visually accurate 3.26 light/dark themes, and it also addresses the following issues: #18, #19, #20.

Yeah, it's a separate add-on project, not a pull request, since it was just a bunch of fixes I was keeping separate originally.

It's designed to sit next to an untouched copy of firefox-gnome-theme and add to/override things as necessary (see its README). Maybe I'll re-organise things later/fork firefox-gnome-theme and merge everything into the fork, don't know.

ghost commented 6 years ago

Nice work! One thing I noticed: if you zoom in on a page, the scrollbar is zoomed as well.

smithfred commented 6 years ago

That was fast ;)

But yeah, I know about the zoom issue:

https://github.com/smithfred/firefox-gnome-theme-3.26-layer/blob/master/userChrome.css#L61

I don't know if there's much that can be done about it without resorting to JS, since there's nothing I could find that exposes the browser zoom level to CSS, and the default "don't scale" behaviour is buried behind -moz-appearance.

ghost commented 6 years ago

Oh, I totally missed that. I'll see it as a feature to indicate page zoom.

smithfred commented 6 years ago

Additional improvement now: auto-detects whether CSD is enabled and the number of window controls.

lunakurame commented 6 years ago

Hi! Sorry for disappearing for quite a while, I didn't have much time for my open source projects recently. Thanks for all those new features and fixes, I'll try to merge them with this repo, unless you want to create pull requests instead?

The general rule is everything which matches normal GTK behavior and doesn't break anything goes to theme.css and all extra features need to be disabled by default and manually enabled in customChrome.css by the user. Everything needs to be opt-in, because there is no way to opt-out in CSS. For example your repo enables some features by default by @importing them in userChrome.css and there is no way to turn them off. So keep that in mind if you decide to add features in a pull request.

That CSD auto-detect code is pretty clever, good job. :D Do you know if it works for both Fedora's FF 58 and normal FF 59?

smithfred commented 6 years ago

My repo's overriding a bunch of stuff since it's just layering on this one, so I'd need to incorporate them into a clone of this repo first.

Also the FF58 fixes are similarly layered on via a separate file; nothing else is touched except the default imports and one extra one in userChrome.css, from memory.

The CSD detection from the first commit of my repo doesn't work past FedFox 57 - the state of "CSD available" was determined by Fedora's about:config key in their 57. FedFox 58 just backports the upstream CSD implementation, which always reports "CSD available" - the FF58 fixes file has info on the updated detection approach, which should also work in vanilla 59 (which Fedora should also end up using). The condition in userChrome.css avoids importing the CSD stuff for vanilla 58 though.

I think it's a good idea to decide on a branching/versioning policy BTW - branches for diff. FF versions (Fedora's CSD stuff/backporting is detectable so doesn't need to be treated separately), and tags/releases following FF's versioning, probably. Since 57 is obsolete now, a working version can be branched and tagged then ignored :D

smithfred commented 6 years ago

BTW depending how much time you have for this project in future, you might want to think about how to allow the project to keep moving forward (other contributors, looking again at the integration team suggestion, etc.)

lunakurame commented 6 years ago

Okay, thanks. I started merging your changes. There are many unrelated changes in a single commit (the initial commit in your repo) and seems like your editor reformatted all the whitespace (tabs → 4 spaces), even lines which weren't changed, so the diffs are messy. I gotta format it back and compare, it's gonna take some time. Most of that code works for FF 57 too, I think I'll just release a final version of this theme for FF 57 after this and we can move on to FF 58.

The GNOME Integration Team didn't answer me last time, looks like they are busy too. I'll continue maintaining this project for now and others can contribute through pull requests. It's not much work to merge a pull request if it's properly formatted (one feature or a bug fix per pull request) and I'd like to review every commit here to understand the changes. Don't worry, I'll let you know if I decide to abandon it, so someone else will take over this project, maybe even you. ;p

smithfred commented 6 years ago

Well the explanation for it being a big initial commit is in the first post on this issue ;)

I replaced tabs with spaces in my proj. because tabs are the work of the devil ;) But it's mostly my own CSS except for one file I copied because it was unavoidable if I wanted to avoid modifying the original project files, I think. Though if it takes a while for you to convert back to evil indentation, I'd be a bit worried by your editor's (non-)capabilty ;)

Oh wait, you mean the 3.26 vs. 3.18 themes? I started with 3.18 light, made changes, then synthesised a new 3.26 dark theme from 3.18 dark and 3.26 light, I think... can't remember. I can check my private repo if it's really useful to know (the GH Git project is not my "real" one because I'm a nutcase and don't like the GH monoculture :D)

The concern is that with a SPOF, if you're gone for another month (which you're entitled to do!) then things grind to a halt. I'll follow up on the Integration Team ticket.

lunakurame commented 6 years ago

Tabs are "evil" only if you use Emacs instead of a real editor, so if your editor can't handle tabs, I guess your editor is the problem, not mine. ;p I can convert spaces to tabs, but it's not 100% accurate because spaces are not semantic indentation characters. Tabs are semantically for indentation, spaces are for alignment, then you can display the code with whatever tab width you want and it's gonna always work properly. But it doesn't matter for this project since I don't align CSS anyway, just indent. Besides, I noticed you misspelled "color" as "colour" in your 3.26 themes, so I corrected it back to "color"~

Wait, you made the 3.26 dark theme from 3.18 dark instead of the 3.26 which was already available in my repo? The original 3.26 dark theme was created by @rafaelmardojai, I have no way to check it since I don't use GNOME 3.26, so I trust your version is accurate too. The code seems to be way different. I don't think I need your private repo's history tho, I already started merging a snapshot of your repo, so I'll figure it out. But thanks for the offer.

It's open source and I picked the most open license possible - public domain. There really is no SPOF, if I get hit by a bus tomorrow, you can just fork it and move on. 4 people already forked it and several hundreds more cloned it. GitHub teams don't really help, according to what I read on GitHub's support pages, still just a single person owns the repo, not the whole team. And that's not a native git feature, I thought you didn't like the GitHub monoculture. >:D

ghost commented 6 years ago

'Color' and 'colour' are both correct. As long as a consistent spelling is used, it's all okay.

lunakurame commented 6 years ago

I know, I just said that in response to claiming tabs are "the work of the devil". I noticed @smithfred changed all occurrences of "color" to "colour", as well as all tabs to spaces, so I thought that was pretty funny. :D

rafaelmardojai commented 6 years ago

I am currently using @smithfred 3.26 theme since i was too busy to finish mine xD. And works properly.

Pd: Github Organizations can have multiple owners =v. Pd2: Please don't remove tabs... legions of spaces everywhere are the real devil... i will pray for it ='u.

smithfred commented 6 years ago

Tabs are "evil" only...

I called them "work of the devil" winking at the fact that tabs vs. spaces is an intractable holy war, don't take it too seriously. Reality is they both have advantages/disadvantages.

Besides, I noticed you misspelled "color" as "colour" in your 3.26 themes

It's OK, I no Americans kan't spel. I won't hold it against you ;)

Wait, you made the 3.26 dark theme from 3.18 dark instead of the 3.26 which was already available in my repo?

No, I made 3.26 light from 3.18 light, then 3.26 dark from my 3.26 light since the 3.26 dark was broken for me.

so I trust your version is accurate too.

Not pixel-perfect but much more accurate than the other one on Fedora/FF57.

There really is no SPOF

Just in terms of momentum

4 people already forked it

Yep and did sod-all after they did ;)

GitHub teams don't really help

See other issue

I thought you didn't like the GitHub monoculture. >:D

Yep, more of Satan's work. Repent!

lunakurame commented 6 years ago

Ok, I'll use @smithfred's version of that theme then, thanks.

I actually tested GitHub Organizations a while ago, the repo owner was still just a single person and others were Collaborators (push access), but that's exactly the same you can do with regular repos. Not sure if they changed it tho. But that's kinda off-topic, I'll focus on merging all that code now.

smithfred commented 6 years ago

Just tested Firefox 58 with the current version of the 3.26 light theme in this project without my fixes; quite a few things wrong vs. before your recent commits with my layer added; some is stuff I fixed a couple of weeks ago on my layer for FF58 (see #22); some not:

Fixed by importing my firefox-58-fixes.css after your main userChrome:

Not fixed by importing that file:

Also TBH the toolbar looks much nicer with GNOME's native icons as available in my additions (reload, new tab, bookmarks, overflow, hamburger) ;)

lunakurame commented 6 years ago

Just merged your theme.css overrides, except the part with the icon filter hack, since I don't understand your changes, and I changes a few minor things too. This is probably the final version for FF57, I'll edit README later, maybe apply some quick bugfixes if I find some and then I'll create a firefox-57 tag or something like that, so we can forget about FF 57 and merge those fixes for FF 58.

smithfred commented 6 years ago

Which "icon filter hack" bit(s) specifically?

Haven't tested since your latest commits; I'll test once you move on to a current Firefox version ;)

lunakurame commented 6 years ago

Which "icon filter hack" bit(s) specifically?

All the stuff related to inverting icon colors you changed. But nevermind, I updated the repo to FF 58 and then realized I'm late again coz there already id FF 59... I think I fixed all of those bugs on your list except for the popup background color (they don't seem to react to my styling attempts) and CSD stuff (looks like Fedora removed FF 58 from their repos?), so I'll just ignore it, tag 58 in like 2 days and move on to 59. I've got more free time now so I want to make this repo up to date again.

I'm finally closing this since there probably won't be more fixes for FF 58 anyway.

smithfred commented 6 years ago

I'll try and re-test your latest commits when I get a chance then.