That'd make it a lot easier to do colorspace analysis.

I added this as a comment in src/web/index.js in #59. Would be good to find a better place for it.

/**
 * Some notes on the startup flow, here:
 *
 * If there's no ?theme param, just ask for a current theme from the add-on. If
 * the add-on is not installed, this does nothing.
 *
 * If there is a ?theme param, that's considered a theme shared with the user.
 * The shared theme is stored as "pending" and also loaded up into the editor.
 * The add-on is then also asked for a current theme.
 *
 * If the add-on never responds with a current theme, the shared theme just
 * appears in the editor.
 *
 * If the add-on is installed and responds with a current theme, then that
 * current theme is loaded into the editor. 
 *
 * If the shared theme is not identical to the current theme, the "pending"
 * shared theme is pressented in an approval dialog with a preview (i.e.
 * SharedThemeDialog). From there, the user can apply the shared theme to
 * override the current theme or skip it and discard.
 *
 * For more details on this flow, check out src/lib/store.js - pay particular
 * attention to the logic involved in the shouldOfferPendingTheme selector.
 */

As discovered in #69, our theme equality checks can fall prey to tricky floating point math precision problems. Could we add some margin for error in that? Two ideas:

1. Flooring each number before comparison.
2. Instead of oldTheme.propertyName === newTheme.propertyName, use something like Math.abs(oldTheme.propertyName - newTheme.propertyName) < 2 to provide a bit of margin for error?

Risk Management

• The service must have performed a Rapid Risk Assessment and have a Risk Record bug
• Public staging and production endpoints must be added to the security baseline

Infrastructure (for ops)

• Access and application logs must be archived for a minimum of 90 days
• Use Modern or Intermediate TLS
• Set HSTS to 31536000 (1 year)
  • strict-transport-security: max-age=31536000
• Set HPKP to 5184000 (60 days)
  • Public-Key-Pins: max-age=5184000; pin-sha256="WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18="; pin-sha256="r/mIkG3eEpVdm+u/ko/cwxzOMo1bk4TyHIlByibiA5E="; pin-sha256="YLh1dUR9y6Kja30RrAn7JKnbQG/uEtLMkBgFF2Fuihg="; pin-sha256="sRHdihwgkaib1P1gxX8HFszlD+7/gTfNvuAybgLPNis=";
    • Start with max-age set to 5 minutes (max-age=300) and increase progressively
    • The first two pins are for Digicert EV and DV roots, the last two are for Let's Encrypt X3 and X4 intermediates (LE is only used for backup)
  • If the service is not hosted under services.mozilla.com, it must be manually added to Firefox's preloaded pins.
• If service has an admin panels, it must:
  • [x] only be available behind Mozilla VPN (which provides MFA)
  • [x] require Auth0 authentication

Development

• Sign all release tags, and ideally commits as well
  • Developers should configure git to sign all tags and upload their PGP fingerprint to https://login.mozilla.com
  • The signature verification will eventually become a requirement to shipping a release to staging & prod: the tag being deployed in the pipeline must have a matching tag in git signed by a project owner. This control is designed to reduce the risk of a 3rd party GitHub integration from compromising our source code.
• Keep 3rd-party libraries up to date
  • Use NSP or [GreenKeeper](https://greenkeeper.io/ Greenkeeper) for NodeJS applications
  • For Python applications, enable pyup security updates:
    • Add a pyup config to your repo (example config: https://github.com/mozilla-services/antenna/blob/master/.pyup.yml)
    • From the "add a team" dropdown for your repo add the relevant "Approved Mozilla PyUp Configuration" team for your github org (e.g. for mozilla and mozilla-services) and grant it write permission.
    • Notify [email protected] to enable the integration in pyup
    • Consider using pip list --outdated or requires.io too
• Integrate static code analysis in CI, and avoid merging code with issues
  • Javascript applications should use ESLint with the Mozilla ruleset
  • Python applications should use Bandit
  • Go applications should use the Go Meta Linter
  • Use whitelisting mechanisms in these tools to deal with false positives

Dual Sign Off

• Services that push data to Firefox clients must require a dual sign off on every change, implemented in their admin panels
  • This mechanism must be reviewed and approved by the Firefox Operations Security team before being enabled in production

Logging

• Publish detailed logs in mozlog format (APP-MOZLOG)
  • Business logic must be logged with app specific codes (see FxA)
  • Access control failures must be logged at WARN level

Security Headers

• Must have a CSP with
  • a report-uri pointing to the service's own /__cspreport__ endpoint
  • web API responses should return default-src 'none'; frame-ancestors 'none'; base-uri 'none'; report-uri /__cspreport__ to disallowing all content rendering, framing, and report violations
  • if default-src is not none, frame-src, and object-src should be none or only allow specific origins
  • no use of unsafe-inline or unsafe-eval in script-src, style-src, and img-src
• Web APIs must set a non-HTML content-type on all responses, including 300s, 400s and 500s
• Set the Secure and HTTPOnly flags on Cookies, and use sensible Expiration
• Make sure your application gets an A+ on the Mozilla Observatory
• Verify your application doesn't have any failures on the Security Baseline.
  • Contact secops@ or ping 'psiinon' on github to document exceptions to the baseline, mark csrf exempt forms, etc.
• Web APIs should export an OpenAPI (Swagger) to facilitate automated vulnerability tests

Security Features

• Authentication of end-users should be via FxA. Authentication of Mozillians should be via Auth0/SSO. Any exceptions must be approved by the security team.
• Session Management should be via existing and well regarded frameworks. In all cases you should contact the security team for a design and implementation review
  • Store session keys server side (typically in a db) so that they can be revoked immediately.
  • Session keys must be changed on login to prevent session fixation attacks.
  • Session cookies must have HttpOnly and Secure flags set.
  • For more information about potential pitfalls see the OWASP Session Management Cheet Sheet
• Access Control should be via existing and well regarded frameworks. If you really do need to roll your own then contact the security team for a design and implementation review.

Databases

• All SQL queries must be parameterized, not concatenated
• Applications must use accounts with limited GRANTS when connecting to databases
  • In particular, applications must not use admin or owner accounts, to decrease the impact of a sql injection vulnerability.

Common issues

• User data must be escaped for the right context prior to reflecting it
  • When inserting user generated html into an html context:
    • Python applications should use Bleach
    • Javascript applications should use DOMPurify
• Apply sensible limits to user inputs, see input validation
  • POST body size should be small (<500kB) unless explicitely needed
• When managing permissions, make sure access controls are enforced server-side
• If handling cryptographic keys, must have a mechanism to handle quarterly key rotations
  • Keys used to sign sessions don't need a rotation mechanism if destroying all sessions is acceptable in case of emergency.
• Do not proxy requests from users without strong limitations and filtering (see Pocket UserData vulnerability). Don't proxy requests to link local, loopback, or private networks or DNS that resolves to addresses in those ranges (i.e. 169.254.0.0/16, 127.0.0.0/8, 10.0.0.0/8, 100.64.0.0/10, 172.16.0.0/12, 192.168.0.0/16, 198.18.0.0/15).

Still kind of baffled by how the theme data ended up being bad from chuck's screengrab. Did a quick patch that seemed to fix the error, but I think the error was unrelated to that PR and symptomatic of a larger issue.

And I think that larger issue is that there's nothing to validate incoming theme JSON data or swap in a known good default if that data is bad. Should fix that with a graceful fallback

We should probably hook up Circle-CI (or Travis-CI) so that we can run [at a minimum] ESLint and maybe build the extension on each PR so we know everything is kosher.

❤️

From acceptance criteria:

• It should have a minimal, pleasant mobile experience that lets users know it's a desktop experiment.

Let's make the page look pretty from the jump by having a cool loader component until we can determine if the user has the add-on available.

I made a pretty rad rainbow theme but noticed that when I select an item to customize, the text grow comically large (FF Nightly), causing the text to wrap and sometimes the circular color chip will squish (see Yellow, Green, Blue, and Purple chips).

As a user, i would like to select from complete example themes.

This feature is laid out in the spec as 'Popular' themes, but should actually just be a set of complete theme declarations we provide.

This page shows the picker UI: https://mozilla.github.io/testpilot-assets/ThemesRFun/01_Measurements/ThemesRFun/#artboard5

This page shows the hover state: https://mozilla.github.io/testpilot-assets/ThemesRFun/01_Measurements/ThemesRFun/#artboard4

This page shows the selected state: https://mozilla.github.io/testpilot-assets/ThemesRFun/01_Measurements/ThemesRFun/#artboard3

From acceptance criteria:

• It should be functionally responsive down to 640px.

Another final checklist item for near the end

From acceptance criteria:

• It should let users reset their browser back to it's default theme state.

The current SVG is all messed up. Please provide a cleaned up version

If an installed user receives a new theme link, they should be prompted to accept it before their browser's ui gets updated.

Spec here:
https://mozilla.github.io/testpilot-assets/ThemesRFun/01_Measurements/ThemesRFun/

If a user installs TRF from the TRF website, they should be prompted to check out test pilot.

See https://hacks.mozilla.org/2018/01/a-rule-based-framework-to-create-dynamic-themes/

This could be an interesting thing for this experiment to do.

Not sure exactly what's going on here but...

Currently, all deployments of the site via the development branch attempt to sign the add-on build. This will fail if the add-on version hasn't been changed.

But, if nothing has changed in the add-on (i.e. no code, not the shared BG images), this is wasteful. Would be handy to tweak the deployment to just download and re-use an existing signed XPI from AMO. (We do this on Test Pilot, as an example.)

We need an add-on icon at both 32 x 32px SVG and a 192 x 192 white PNG (for the Test Pilot Site)

Only the Toolbar Color and Toolbar Input Background Color accept alpha values. The color picker UI should omit alpha values for other color types.

If a user visits TRF from a non-firefox browser, we should prompt them to download firefox:

Spec is here: https://mozilla.github.io/testpilot-assets/ThemesRFun/01_Measurements/ThemesRFun/#artboard2

Note: do not implement the dismiss button

URL should be something like this with the final name slotted in:

https://www.mozilla.org/firefox/new/?utm_campaign=${tbd}-acquisition&utm_medium=referral&utm_source=${tbd}.firefox.com

Not sure if we need to use something like cross-env for cross-OS greatness when passing ENV vars via the CLI. Some potential edges are:

$ cat package.json | jq '. | {scripts}' | grep "="

1. "server": "NODE_ENV=development webpack-dev-server --config webpack.web.js",

2. "watch:web": "NODE_ENV=development webpack --watch --progress --colors --config webpack.web.js",

3. "watch:extension": "NODE_ENV=development webpack --watch --progress --colors --config webpack.extension.js",

4. "build:web": "NODE_ENV=production webpack -p --config webpack.web.js",

5. "build:extension": "NODE_ENV=production webpack -p --config webpack.extension.js",

6. "package": "npm run clean:extension && npm run build:extension && web-ext build --overwrite-dest --source-dir=build/extension && mv web-ext-artifacts/*.zip ./addon.xpi",

7. "sign": "npm run clean:extension && npm run build:extension && web-ext sign --source-dir=build/extension && mv web-ext-artifacts/*.xpi ./addon.xpi",

8. "run": "npm run build:extension && web-ext run --source-dir=build/extension",

9. "release:dev": "ADDON_URL='https://lmorchard.github.io/ThemesRFun/addon.xpi' SITE_URL='https://lmorchard.github.io/ThemesRFun/' npm run release:base"

From acceptance criteria:

• It should have a list of mandatory links at the bottom of the page including any required privacy documentation.

Assets: https://github.com/mozilla/testpilot-assets/tree/master/ThemesRFun/00_Assets
Measurements: https://mozilla.github.io/testpilot-assets/ThemesRFun/01_Measurements/ThemesRFun/

Thanks!

From acceptance criteria:

• It should let users save themes they've generated to localStorage and see previews of them on subsequent visits.

Just a quick check. I think everything looks OK to my aging eyes, but worth a second look with somebody that has more than 20 minutes of experience with this repo:

$ npx depcheck

Unused dependencies
* json-url

Unused devDependencies
* babel-core
* babel-loader
* babel-plugin-transform-object-rest-spread
* babel-preset-env
* babel-preset-node8
* babel-preset-react
* clean-webpack-plugin
* css-loader
* eclint
* ejs
* eslint-config-airbnb-base
* eslint-plugin-import
* eslint-plugin-react
* image-webpack-loader
* node-sass
* sass-loader
* style-loader
* svg-url-loader
* url-loader

json-url felt fishy, but looks like we're manually copying the file from ./node_modules/ into a /vendor/ directory (which is cool).

Babel and ESLint and *-loader results are always fishy, based on how they are loaded by their respective tools.

So I think maybe the only one that strikes me was eclint. I didn't see any errors when running npx eclint . locally, so I'm guessing there were no editorconfig errors. Not sure if we want to create some npm task which calls eclint or not. We could potentially extend npm run lint to run ESLint and eclint (and some Sass linter if we feel under-linted).

blah blah blah

$ npx depcheck --ignores=*-loader,eslint-*,babel-*

Unused dependencies
* json-url

Unused devDependencies
* clean-webpack-plugin
* ejs
* node-sass

Similar to send.firefox.com

@youwenliang

From acceptance criteria:

• All buttons and links should have visible focus states
• All buttons and links should be accessible via keyed entry (tab selection)
• All form elements should include appropriate label attributes
• All grouped buttons should be nested in a and described with a legend
• All UI should be verified to use A11y friendly contrast ratios

Filing this issue as a checklist item for near the end, though it should be an aspect we mind all along the way

From These Weeks in Firefox: Issue 32 :

The WebExtension theme API now supports specifying colors for frame_inactive, tab_loading, icons and icons_attention, button_background, button_background_hover and button_background_active, toolbar_field_separator.

Need to play with these and see exactly what they do, filing this issue to not forget about it.

We should have a /__version__ resource like Test Pilot that gets generated on deployment. Properties something like this:

{
  "commit": "3cfbd582b52caaf0b4c032d15b5d886a7827ec31",
  "version": "2018-02-06",
  "source": "https://github.com/mozilla/ThemesRFun"
}

Not sure if these came up in the a11y linting bug results from sonarwhal or lighthouse tools in #47 (comment) (or anywhere else), but...

Do we want to add Twitter and OpenGraph meta tags for improved sharing greatness?

<head>
  <meta charset="utf-8">
  <title>Firefox Theme Generator</title>
  <link rel="icon" type="image/svg+xml" href="images/icon.svg">
  <link href="index.css" rel="stylesheet">
  <script type="text/javascript" charset="utf-8" async="" src="https://mozilla.github.io/ThemesRFun/vendor/json-url-msgpack.js"></script>
  <script type="text/javascript" charset="utf-8" async="" src="https://mozilla.github.io/ThemesRFun/vendor/json-url-lzma.js"></script>
  <script type="text/javascript" charset="utf-8" async="" src="https://mozilla.github.io/ThemesRFun/vendor/json-url-safe64.js"></script>
</head>

From acceptance criteria:

• It should ping GA for all pings described in the experiment metrics doc.
• It should respect DNT.

If i pull off a tab into a new window I lose my TRF theme in the new window.

Maybe this is a limitation of the API, but it'd be good to know if this behavior is preventable.

Could use CircleCI to auto-deploy to gh-pages with some credentials in env vars and a tweak to add the git hash to add-on version number when running npm run release:dev.

Also might be worth creating a development branch, so auto-deploy happens from there rather than master

Survey link should only appear when the user has TRF installed
Survey link should have the following params:

ver=${addon-ver} and ref=app

I used a JS object for message handler dispatch. This should probably be a switch statement to constrain the accepted messages:

https://github.com/mozilla/ThemesRFun/blob/master/src/extension/background.js#L20

        messageHandlers[message.type]({ port, message })
    );
    port.postMessage({ type: 'hello' });
  });
  fetchTheme().then(applyTheme);
};

const messageHandlers = {
fetchTheme: ({ port }) => {

In #7, I got auto-deployment to gh-pages working from the development branch - not master. That means any deploy to dev has to go through that branch first. This differs from Test Pilot proper.

Apropos of that difference, it might be nice to make every merge to master get deployed.

This is practically blocked by #21 - since there will be an error in add-on signing during deployment unless the add-on version is changed on every commit.

... Turns out, I was invoking eclint incorrectly, and the src/images/*.svg files are throwing some errors:

$ npx eclint check src/**

src/images/back-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

src/images/bookmark-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

src/images/close-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

src/images/forward-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

src/images/home-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

src/images/info-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

src/images/menu-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

src/images/more-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

src/images/refresh-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

src/images/sidebar-16.svg
    02:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)
    03:01 ❌️ invalid indent size: 3, expected: 4 (EditorConfig indent_size https://goo.gl/QtGB8R)

Here's a random .SVG file, with the 3 space indented copyright header:

<!-- This Source Code Form is subject to the terms of the Mozilla Public
   - License, v. 2.0. If a copy of the MPL was not distributed with this
   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16">
  <path d="M15 7H3.414l4.293-4.293a1 1 0 0 0-1.414-1.414l-6 6a1 1 0 0 0 0 1.414l6 6a1 1 0 0 0 1.414-1.414L3.414 9H15a1 1 0 0 0 0-2z"></path>
</svg>

Not sure if we need the copyright headers, or if we just run the whole mess through SVGO and strip the comments.

I randomly tried adding the eslint-plugin-jsx-a11y plugin to our ESLint config, extended the "plugin:jsx-a11y/recommended" rules and it returned 18 errors. Not sure if we want to take a look and possibly add it to our config:

$ npm run lint

> [email protected] lint /Users/pdehaan/dev/github/mozilla/ThemesRFun
> eslint --color .

/Users/pdehaan/dev/github/mozilla/ThemesRFun/src/web/lib/components/BrowserPreview/index.js
   33:5   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   33:5   error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions
   41:5   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   41:5   error  Non-interactive elements should not be assigned mouse or keyboard event listeners               jsx-a11y/no-noninteractive-element-interactions
   49:7   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   49:7   error  Non-interactive elements should not be assigned mouse or keyboard event listeners               jsx-a11y/no-noninteractive-element-interactions
   67:7   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   67:7   error  Non-interactive elements should not be assigned mouse or keyboard event listeners               jsx-a11y/no-noninteractive-element-interactions
   79:7   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   79:7   error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions
   89:9   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   89:9   error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions
  102:11  error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
  102:11  error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions

/Users/pdehaan/dev/github/mozilla/ThemesRFun/src/web/lib/components/ThemeBackgroundPicker/index.js
  14:5  error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
  14:5  error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions

/Users/pdehaan/dev/github/mozilla/ThemesRFun/src/web/lib/components/ThemeColorsEditor/index.js
  25:15  error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
  25:15  error  Non-interactive elements should not be assigned mouse or keyboard event listeners               jsx-a11y/no-noninteractive-element-interactions

✖ 18 problems (18 errors, 0 warnings)

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] lint: `eslint --color .`
npm ERR! Exit status 1

diff --git a/.eslintrc.yaml b/.eslintrc.yaml
index 2b0df99..c4a961b 100644
--- a/.eslintrc.yaml
+++ b/.eslintrc.yaml
@@ -5,13 +5,18 @@ env:
   webextensions: true

 extends:
-- eslint:recommended
-- airbnb-base
-- plugin:react/recommended
+  - eslint:recommended
+  - airbnb-base
+  - plugin:react/recommended
+  - plugin:jsx-a11y/recommended

 parserOptions:
   ecmaVersion: 6

+plugins:
+  - jsx-a11y
+  - react
+
 root: true

And on that note, not sure if we want to drop the airbnb-base rules in favor of the plugin:mozilla/recommended rules or not.

env:
  browser: true
  es6: true
  node: true
  webextensions: true

extends:
  - eslint:recommended
  - plugin:jsx-a11y/recommended
  - plugin:mozilla/recommended
  - plugin:react/recommended

parserOptions:
  ecmaVersion: 6
  sourceType: module

plugins:
  - jsx-a11y
  - mozilla
  - react

root: true

rules:
  import/no-named-as-default: off
  import/prefer-default-export: off
  react/prop-types: off

  quotes: [error, single]

From acceptance criteria:

• It should generally match Photon styles.

We'll probably just be doing this along the way, but filing this issue as kind of a final checkbox toward the end.

From acceptance criteria:

• It should let users undo or redo the changes they've made in an editing session.

This currently works with browser back & forward navigation. We should be able to hook the same thing up to buttons on the page by maintaining a stack of previous state.

From acceptance criteria:

• It should match final visual specs.

Another final checklist item for near the end

• There's some kind of weird flickering thing happening with the links
• The GH/Twit links don't seem to be clickable
• The footer should always be at the bottom of the window

I am going to round up a bunch of bg/texture images from visual designers.