Hard-coded color codes - needed?

[CarlColijn]

Looking through the css code of the compiled userChrome and userContent files I noticed the nice way to switch colors around using the variables provided in :root. However, in some places hard-coded colors are used, like "color: white" or "color: #ccc". An example: in userContent.css, in @-moz-document url-prefix(about:newtab), url(about:home) the element .search-wrapper input has it's color set to var(--primary-light-color), but it's background to #4a4a4f. I suppose that #4a4a4f could also be modified to var(--grey-60) (which has the same value) to get the same default effect but with better customizability? Or one step more meta: wherever variables like --grey-xx are used in the actual content, they could also be replaced with the e.g. --primary-dark-color variables?

Is there a reason for this? Or is it a left-over from still-not-variablized 1st try code or such? Or maybe a conscious decision not to explode the number of variables needed like --primary-dark-color?

The reason I ask is because I find the default grey still a bit too light, and wanted to modify the theme by altering these variables (to get a more black/white look). Finding these hard-coded values later on in the files I started updating those as well, but it soon dawned on me these hard-coded values might be there for a reason.

[overdodactyl]

Hi @CarlColijn, thanks for bringing this to my attention. More than anything this was an “artifact” of my first versions of the code (before I was really considering making the project a public repo/making it customizable for others).

You can change them without worry, but this is something that should be fixed on my end as well (both removing hard coded hex colors and the use of things like var(—grey-40) after the initial setting of variables).

I should be able to get that taken care of tonight.

[CarlColijn]

Great! But don't take off leave days to work on this on my account - I've temporarily fixed it on my side by changing the relevant colors, and I don't see myself grabbing an updated version in the short term, so no need to hurry from me. IMO it would add value to the project, though, so a nice improvement!

[overdodactyl]

Definitely an improvement! I'm getting close to having them all removed :)

I'm not sure what OS you are on, but you may be interested in making use of the new updater scripts. Part of the goal of these were to make an easy way for users to update, but not have to manually change any color variables or internal UUIDs redefined each time. See #62 for more info

[CarlColijn]

Thanks for that. I'm on Windows 10 and didn't want to introduce extra tool chains I don't already have on this machine (like Gulp, if it's available for Windows at all), so I opted for the pre-built css files.

I see your update script was meant for this use case, but is only for *nix versions; maybe I can be of help writing a Windows version? Pre-installed tools like curl etc. are not an option for regular Windows users (needs to be downloaded first), but there's other mechanisms to use that come out-of-the-box on Windows platforms. The script will thus not be a 1-to-1 port I think, but a separately developed one.

[overdodactyl]

Definitely can understand not wanting to add gulp and all it’s dependencies..I know that’s not an ideal solution for everyone.

Absolutely no pressure, but I would definitely appreciate any help with a Windows versions!

I unfortunately don’t currently have access to a Windows computer, so I’m somewhat limited in my ability to write/test things for it (I also didn’t realize some tools like curl aren’t available by default).

[CarlColijn]

Will do :) I think I'll have something ready before the end of the weekend.

In your bash script I saw you find/replace the --blue-xx parts of the definitions of the --primary-accent-color: var(--blue-xx) with a hard coded color value the user can alter in the upgrade script. Wouldn't it be better to just re-use the two :root definition blocks at the top of the css files? If so, it would be easier if they were delimited with a unique string to do easy find/replace on, like e.g. /* begin user customizable section */ and /* end user customizable section */. The update scripts could then just copy out any existing definitions from the old files, and if found, replace the definitions in the downloaded files with these old ones.

And an extra benefit is that users could then also add extra css directives and styles in there that they want to use; I e.g. also have an extra #fullscr-toggler { display:none!important; } I want to insert.

As an extra question: what is the use of the section

  ## change any webextension Internal UUIDs here.
  ## one example is provided

  ## Define Internal UUID's for extensions here
  uMATRIX_INTERNAL_UUID="32818407-cb70-5d40-9f8d-81ed9f2012a6"

  ## Replace Ineternal UUID placeholders with variables defined above
  sed -i '' "s/9eba7fab-892c-7b42-a57e-b876d4196d70/$uMATRIX_INTERNAL_UUID/" "userContent.css"

? Is it meant to add user-definable UUID replacements for extensions, or is it more for you to specify new UUIDs to use not yet used in the css files?

[overdodactyl]

Awesome, thank you!! And no rush if you don’t have time this weekend :)

I had considered something like replacing the root sections, but I was worried it might cause problems in future versions. Things like var(—primary-accent-color) should stay pretty stable throughout the lifetime of the project, but I think a good way to improve the project would be to increase the customizability of it. To do that, I would need to increase the number of variables used in the root sections. For example, right now anywhere I had to change any code editors/viewers (such as view-source pages), I used the hex colors used by FFs developer toolbox. It would be nice to create variables like —code-blue or —code-attribute-color. If the script just used the old version of the roots and a new variable were added, it wouldn’t get picked up and would cause problems anywhere it was used. You might have a better idea of how to get around this problem though.

I definitely like the idea of having a user custumoziation section though. Ideally it should be places at the end of the file, however. That way a user can not just add to ShadowFirefox, but also override anything in it (as an example, one user wanted more compact context menus than are what are set by ShadowFox. To do so their changes had to go at the end as to not get overridden by what I set).

As for the UUIDs, they were meant to be a way for people to replace the Internal UUID of extensions I made tweaks for. Since the id is different on every install, each user has to get their own and use it in userContent.css, otherwise the changes won’t go into effect. So similar to the color variables, I wanted a way to automatically replace those instead of having to manually do so at every update.

For a little more info, see here:

https://github.com/overdodactyl/ShadowFox/wiki/Altering-webextensions

On that note, I should probably change the UUIDs in ShadowFox from the random id used when I tested it to a variable (like uBlock_uuid or something).

(Sorry for any and all typos - currently on mobile)

[overdodactyl]

To address the customization section there are two routes we could take I think (might be something better than what I’m thinking though):

The first would be to have a sections with a specific header like you mentioned, then just transfer it on an update.

Another route I think might be worth considering is just to recommend users create two files with any of their own tweaks (something like SF_userContent_customization.css...probably need a better name though), then the updated script would simply append those files to the end of userChrome and userContent, respectively.

[CarlColijn]

About the user customizations: a css @import with a fixed file name at the end of the ShadowFox css files could also work. That removes the need for the update script to pull in any content. If the user doesn't provide that file it will get skipped by default, and if it is provided it gets picked up and can override anything in the ShadowFox files.

The same can be done for the :root color definitions; specify another import file the user can create that is @import-ed at the top but below the default :root definitions, which could then be used by the user to override any default color definitions. But only if --var settings in css can be overriden; I have no experience with them.

[CarlColijn]

Ah, update: I quickly tested this, and FireFox on Windows respects @imports when they're the first thing encountered in the userChrome.css file, but anywhere else and they seem to get ignored... Pity if this is truly so, since it would have been a nice integration mechanism. Could you test on Linux or Mac for confirmation?

[overdodactyl]

@imports are unfortunately only respected if they’re at the very top of the file.

Doubly unfortunate, there’s a Linux bug that prevents the usage of import statements (this bug is actually what spurred the usage of gulp as opposed to just having the userChrome/Content files filled with imports users could remove, add to etc - essentially what is now userChrome_imports.css and userContent_imports.css in the repo).

[CarlColijn]

Shame indeed... I've taken the route of updating delimited sections. A simple multi-line regex find/replace for hard coded section headers should do the trick, each file having two sections: a color overrides section right after the :root color sections, and a customization overrides section at the end of the file. The ShadowFox css files should thus incorporate these (empty) sections so the update scripts know where to add the content. I suggest section delimiters like \* ShadowFox start color overrides *\ and \* ShadowFox end color overrides *\, with an analogous for the customizations.

Both approaches you suggest have their own benefits and drawbacks;

Picking up customizations from external files

• +: has the opportunity to only use one color overrides file for both userContent & userChrome
• 0: these override files need to be stored next to the update script, directly in the profile folder (messy?), for new users to be able to make a flying start (define customizations, run update script for the 1st time, chrome folder gets created with correct files in them)
• 0: or put these files in the chrome folder itself, but that necessitates a 2-stage initial install (but won't most users install the vanilla version first, then later start modifying anyway?)

Transferring customized sections from the old css files to the new one

• +: there's no need for extra customization files lying around (clean)
• 0: new users can immediately start modifying after installation; no need for running the updater again to integrate their changes
• -: there's no (clear) way to sync any customizations between userChrome and userContent

Which way do you prefer?

[CarlColijn]

I do have a suggestion for the UUID replacements. In the current set-up, you have in the css files e.g.

@-moz-document url-prefix("moz-extension://9eba7fab-892c-7b42-a57e-b876d4196d70/")  {

In the update script there's this section:

uMATRIX_INTERNAL_UUID="32818407-cb70-5d40-9f8d-81ed9f2012a6"
sed -i '' "s/9eba7fab-892c-7b42-a57e-b876d4196d70/$uMATRIX_INTERNAL_UUID/" "userContent.css"

A more direct approach might be to specify in the css files:

@-moz-document url-prefix("moz-extension://SF_uMATRIX_UUID/")  {

and then in the update script do a

uMATRIX_UUID="32818407-cb70-5d40-9f8d-81ed9f2012a6"
sed -i '' "s/SF_uMATRIX_UUID/$uMATRIX_UUID/" "userContent.css"

This opens the way for the user to be able to specify an external UUID overrides file filled with e.g.

uMATRIX_UUID=32818407-cb70-5d40-9f8d-81ed9f2012a6
other_UUID=...

This in turn makes the update script totally self-contained without needing to be updated by the user; the user only needs to specify the UUIDs of the extensions he uses, and it would be easier for non-technical users to do this (we could add a sample UUID replacement file to get started).

For me under Windows this is easy to add to the installer script (using VBScript), but I have no experience with writing Bash scripts :) Can you grep (or such) the content of a file, split it on newlines and '=' characters, loop on these and in that loop use those to drive a regex replace on the css content?

[overdodactyl]

I'm open to the other way, but I'm kinda leaning towards the option of using separate files for the following reasons:

• I really like your idea regarding the internal UUIDs. Going this route, however, already kind of accepts that we are adding extra customization files - a little less clean, but worth the trade off I think. I like the consistency of "if you want to make customization, use the extra files provided," as opposed to, "if you want customization, make x changes in the extra files and make y changes in userChrome/Content."

• I think it's a huge advantage for users not to have to set the same variables in two locations. For some, I don't think it would be super intuitive to have to make the same changes twice.

• I think it might be easier for new users to make edits in a "clean" file as opposed to opening up and messing with userChrome/Content, which already contains hundreds of lines of code.

• We could place these three files in their own "ShadowFox_Customization" folder inside of the chrome directory. This would keep them out of the "messy" profile directory, and it makes it clear what their purposes are (also easier to keep track of, copy and paste to a new profile etc.). It is slightly unfortunate this would necessitate a 2-step initial install for customization to go into effect, but that's not a huge concern to me because, as you mentioned, I'm guessing most people will want download SF as is, then see what changes they want to make on top of that as opposed to writing overrides prior to install.

[overdodactyl]

^^ If we go this route, we would still need to add something like /*! [Users Ignore] color_Overrides.css placed below */ or something of the sort in both userChrome and userContent for the script could pickup were to place the color overrides

[overdodactyl]

For the internal UUIDs, I think we should just keep the same variable name in userContent.css as what a user would define in their file instead of using, for example, SF_uMATRIX_UUID and uMATRIX_UUID. That way, the script would just need to read the lines of the file and replace what it finds.

This works in a shell script:

while IFS='' read -r line || [[ -n "$line" ]]; do
    IFS='=' read -r -a array <<< "$line"
    sed -i '' "s/${array[0]}/${array[1]}/" "userContent.css"
done < "internal_UUIDs.txt"

[overdodactyl]

So nothing's set in stone (we can still use the other approach), but here's kinda what I was thinking:

#!/bin/bash

### ShadowFox updater for Mac
## author: @overdodactyl
## version: 1.1

userChrome="https://raw.githubusercontent.com/overdodactyl/ShadowFox/master/userChrome.css"
userContent="https://raw.githubusercontent.com/overdodactyl/ShadowFox/master/userContent.css"

echo "\nThis script should be run from inside your Firefox profile.\n"

currdir=$(pwd)

## get the full path of this script (readlink for Linux, greadlink for Mac with coreutils installed)
sfp=$(readlink -f "${BASH_SOURCE[0]}" 2>/dev/null || greadlink -f "${BASH_SOURCE[0]}" 2>/dev/null)

## fallback for Macs without coreutils - may cause problems if symbolic links are encountered
if [ -z "$sfp" ]; then sfp=${BASH_SOURCE[0]}; fi

## change directory to the Firefox profile directory
cd "$(dirname "${sfp}")"

## Make chrome directory if it doesn't exist
mkdir -p chrome;

## Move to chrome directory
cd chrome;

## Make ShadowFox_customization directory if it doesn't exist
mkdir -p ShadowFox_customization;

## Create all customization files if they don't exist
touch ./ShadowFox_customization/colorOverrides.css
touch ./ShadowFox_customization/internal_UUIDs.txt
touch ./ShadowFox_customization/userContent_customization.css
touch ./ShadowFox_customization/userChrome_customization.css

echo "Updating userContent.css and userChrome.css for Firefox profile:\n$(pwd)\n"

if [ -e userContent.css ]; then
  echo "Your current userContent.css file for this profile will be backed up and the latest ShadowFox version from github will take its place."
else
  echo "A userContent.css file does not exist in this profile. If you continue, the latest ShadowFox version from github will be downloaded."
fi

if [ -e userChrome.css ]; then
  echo "Your current userChrome.css file for this profile will be backed up and the latest ShadowFox version from github will take its place.\n"
else
  echo "A userChrome.css file does not exist in this profile. If you continue, the latest ShadowFox version from github will be downloaded.\n"
fi

read -p "Continue Y/N? " -n 1 -r
echo "\n\n"

if [[ $REPLY =~ ^[Yy]$ ]]; then
  if [ -e userChrome.css ]; then
    # backup current userChrome.css file
    bakfile="userChrome.backup.$(date +"%Y-%m-%d_%H%M")"
    mv userChrome.css "${bakfile}" && echo "Your previous userChrome.css file was backed up: ${bakfile}"
  fi
  if [ -e userContent.css ]; then
    # backup current userChrome.css file
    bakfile="userContent.backup.$(date +"%Y-%m-%d_%H%M")"
    mv userContent.css "${bakfile}" && echo "Your previous userContent.css file was backed up: ${bakfile}"
  fi

  # download latest ShadowFox userChrome.css
  echo "downloading latest ShadowFox userChrome.css file"
  curl -O ${userChrome} && echo "\nShadowFox userChrome.css has been downloaded"

  # download latest ShadowFox userContent.css
  echo "downloading latest ShadowFox userContent.css file"
  curl -O ${userContent} && echo "\nShadowFox userContent.css has been downloaded\n"

  if [ -s ./ShadowFox_customization/internal_UUIDs.txt ]; then
    ## Insert any UUIDs defined in internal_UUIDs.txt into userContent.css
    while IFS='' read -r line || [[ -n "$line" ]]; do
        IFS='=' read -r -a array <<< "$line"
        sed -i '' "s/${array[0]}/${array[1]}/" "userContent.css"
    done < "./ShadowFox_customization/internal_UUIDs.txt"
    echo "Your internal UUIDs have been inserted.\n"
  else
    echo "You have not defined any internal UUIDs for webextensions."
    echo "If you choose not to do so, webextensions will not be styled with a dark theme and may have compatibility issues in about:addons."
    echo "For more information, see here:"
    echo "https://github.com/overdodactyl/ShadowFox/wiki/Altering-webextensions\n"
  fi

  if [ -s ./ShadowFox_customization/colorOverrides.css ]; then
    ## Delete everything inbetween override markers
    sed -i '' '/\/\*! Begin color overrides \*\//,/\/\*! End color overrides \*\//{//!d;}' userContent.css
    sed -i '' '/\/\*! Begin color overrides \*\//,/\/\*! End color overrides \*\//{//!d;}' userChrome.css

    ## Insert everything from colorOverrides.css
    sed -i '' '/\/\*! Begin color overrides \*\// r ./ShadowFox_customization/colorOverrides.css' userContent.css
    sed -i '' '/\/\*! Begin color overrides \*\// r ./ShadowFox_customization/colorOverrides.css' userChrome.css

    echo "Your custom colors have been set.\n"
  else
    echo "You are using the default colors set by ShadowFox"
    echo "You can customize the colors used by editing colorOverrides.css\n"
  fi

  if [ -s ./ShadowFox_customization/userContent_customization.css ]; then
    ## Append tweaks to the end of userContent.css
    cat ./ShadowFox_customization/userContent_customization.css >> userContent.css
    echo "Your custom userContent.css tweaks have been applied.\n"
  else
    echo "You do not have any custom userContent.css tweaks."
    echo "You can customize userContent.css using userContent_customization.css.\n"
  fi

  if [ -s ./ShadowFox_customization/userChrome_customization.css ]; then
    ## Append tweaks to the end of userContent.css
    cat ./ShadowFox_customization/userChrome_customization.css >> userChrome.css
    echo "Your custom userChrome.css tweaks have been applied.\n"
  else
    echo "You do not have any custom userChrome.css tweaks."
    echo "You can customize userChrome.css using userChrome_customization.css.\n"
  fi

else
  echo "Process aborted"
fi

## change directory back to the original working directory
cd "${currdir}"

any thoughts?

[CarlColijn]

Looks good! I'd say go ahead and publish your version without waiting for me; it's a good improvement anyway. I can adapt perfectly fine to your modifications.

I've already updated my script with your changes. I don't have the time now to properly test it as well since we've got family over today who are getting impatient :), but you can expect my (initial) version this evening or tomorrow.

The only thing I'm still doubting a bit on is the way to give user feedback from my script. I've decided to utilize VBScript (.vbs files), since regular Windows users are somewhat uncomfortable with a command line, and VBScript enables you to show proper message boxes (plus that VBScript offers a true programming environment). Then again there's a lot of text to communicate; a simple message box might not be sufficient to comfortably show it all as-is. I'll ponder some more on this. Maybe I'll have to condense the texts in your version a bit to make them usable in my script.

[overdodactyl]

Sounds good! And truly, no rush! Don't miss out on family time on my account :)

I'm not too familiar with VBS, so I unfortunately don't have any great insights there haha. Feel free to change what you need though, I'm not necesarilly sure what I have is the best route anyways, just kinda the first thing that came to mind.

If you don't have a lot of space, one possibility is just to link to a wiki page on here with any necessary instructions or what not.

[CarlColijn]

A little detail that's different in my install script that you might want to add to the other install scripts as well: when testing, I got an error on creating the backup files. The original code used a yyyy-mm-dd_hh-mm timestamp, so I just added -ss to the end. I don't expect users to be that impatient when trying a new install, but you never know :)

[overdodactyl]

That's a good idea :)

On the backups note, I was pondering whether or not they should be put in a backups folder instead of just the chrome folder. After enough updates, that could create a pretty messy folder filled with files not likely to be accessed very often.

[overdodactyl]

So getting back to the original issue here of hard coded variables, here's a quick update:

I've been working on a different branch of the repo, which can be found here:

https://github.com/overdodactyl/ShadowFox/tree/variable_replacement

I've made some changes that should help quite a bit. To start, I've made much better use of FF's built in variables (mostly those redefined in color_variables.css). In doing so, I was able to cut a significant amount of code in the repo. Additionally, this will make the visual appearance across pages a lot more consistent (previously, there were some variations from page to page in things like button colors, borders etc.).

I ended up changing the variable names from:

--primary-light-color: var(--grey-40);
 --light-accent-color: var(--grey-50);
 --mid-way-color: var(--grey-60);
--primary-dark-color: var(--grey-70);
--dark-accent: var(--grey-80);
 --darksest: var(--grey-90);
--primary-accent-color: var(--blue-40);
--primary-accent-color-dark: var(--blue-50);
--primary-accent-color-darkest: var(--blue-60);

to

  --tone-1: var(--grey-10);
  --tone-2: var(--grey-20);
  --tone-3: var(--grey-30);
  --tone-4: var(--grey-40);
  --tone-5: var(--grey-50);
  --tone-6: var(--grey-60);
  --tone-7: var(--grey-70);
  --tone-8: var(--grey-80);
  --tone-9: var(--grey-90);
  --accent-1: var(--blue-40);
  --accent-2: var(--blue-50);
  --accent-3: var(--blue-60);

I know this will annoy some people initially, but I think it was well worth the change. The previous names I was using were a bit confusing, and they prevented me from using more variables (i.e. grey-10 --> 90). If someone want to make things darker, all they would have to do is bump up the tone values.

For another layer of customization, all code in the repo now uses the second set of variables after these are defined (lines 66 -> 106). This way, users can have more granular control. For example, if someone wanted their main buttons to be a different color than their accent color, they would simply have to change the value of --in-content-primary-button-background.

I've also made better use of some code that was duplicated in bother userChrome and userContent (new common-files directory). This also lead to an easy fix for some un-styled/ugly appearances in the about:preferences dialogue boxes (such the cookies menu, where things previously had an inverted color).

Lastly, I started using clean-css to help "beautify" the code.

All together, these changes reduced the code base by over 2,000 lines (!!).

There are a few exceptions to what's outlined above (I haven't yet gotten to replacing all --tone-x variables with other variables in the webextensions yet, for example).

I'll open a new issue when these changes go "live" in the master branch to make everyone aware of the variable changes, but just wanted to give a quick update. And if anyone wants to give the variable_replacement branch a test and let me know if everything looks good, that would be great :+1:.

[overdodactyl]

Oh, and @CarlColijn, due to a weird quirk in clean-css, I had to change the indicators that the scripts utilize from

/*! Begin color overrides */
/*! End color overrides */

to

--start-indicator-for-updater-scripts: black;
--end-indicator-for-updater-scripts: black;

[CarlColijn]

All good things! I think the benefits outweigh the inconveniences for users. I assume clean-css strips / / comments?

[overdodactyl]

It does strip /* */ comments, but it keeps them if you use the format /*! */.

Using the formatting option, you can specify whether or not there should be line breaks after a comment, but those settings aren't respected if you're in a root qualifier for some (undocumented) reason - probably worth opening a new issue, but didn't want to wait on that. So

/*! Begin color overrides */
/*! End color overrides */
--in-content-page-color: var(--tone-4)!important;
--in-content-page-background: var(--tone-7)!important;

would become

/*! Begin color overrides *//*! End color overrides */--in-content-page-color: var(--tone-4)!important;
--in-content-page-background: var(--tone-7)!important;

no matter the settings.

[overdodactyl]

I think the hard coded values have all been removed - feel free to let me know if you run across any others