Merge pull request #638 from ueokande/qa-0.24

QA 0.24
2019-09-03 03:11:10 +00:00 · 2019-09-03 03:11:10 +00:00 · ab6295d0f1
commit ab6295d0f1
parent 07bcc15e7b f64b7ca320
16 changed files with 1514 additions and 5269 deletions
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@ -75,10 +75,6 @@ jobs:
      - checkout
      - setup_npm
      - run: npm run build
-      - run:
-          name: Run geckodriver
-          command: geckodriver
-          background: true
      - run: npm run test:e2e

 workflows:
--- a/.eslintrc
+++ b/.eslintrc
@ -31,6 +31,7 @@
    "default-case": "off",
    "dot-location": ["error", "property"],
    "function-paren-newline": "off",
+    "function-call-argument-newline": ["error", "consistent"],
    "id-length": "off",
    "indent": ["error", 2],
    "init-declarations": "off",
--- a/QA.md
+++ b/QA.md
@ -25,46 +25,10 @@ The behaviors of the console are tested in [Console section](#consoles).

 ### Settings

-#### JSON Settings
-
-##### Validations
-
- [ ] show error on invalid json
- [ ] show error when top-level keys has keys other than `keymaps`, `search`, `blacklist`, and `properties`
-
-###### `"keymaps"` section
-
- [ ] show error on unknown operation name in `"keymaps"`
-
-###### `"search"` section
-
- validations in `"search"` section are not tested in this release
-
-##### Updating
-
- [ ] changes are updated on textarea blure when no errors
- [ ] keymap settings are applied to open tabs without reload
- [ ] search settings are applied to open tabs without reload
-
-##### Properties
-
- [ ] show errors when invalid property name
- [ ] show errors when invalid property type
-
 #### Form Settings

-<!-- validation on form settings does not implement in 0.7 -->
-
-##### Search Engines
-
- [ ] able to change default
- [ ] able to remove item
- [ ] able to add item
-
 ##### `"blacklist"` section

- [ ] able to add item
- [ ] able to remove item
 - [ ] `github.com/a` blocks `github.com/a`, and not blocks `github.com/aa`

 ##### Updating
@ -72,12 +36,6 @@ The behaviors of the console are tested in [Console section](#consoles).
 - [ ] keymap settings are applied to open tabs without reload
 - [ ] search settings are applied to open tabs without reload

-### Settings source
-
- [ ] show confirmation dialog on switched from json to form
- [ ] state is saved on source changed
- [ ] on switching form -> json -> form, first and last form setting is equivalent to first one
-
 ## Find mode

 - [ ] open console with <kbd>/</kbd>
@ -86,9 +44,3 @@ The behaviors of the console are tested in [Console section](#consoles).
 - [ ] Wrap search by <kbd>n</kbd>/<kbd>N</kbd>
 - [ ] Find with last keyword if keyword is empty
 - [ ] Find keyword last used on new tab opened
-
-## Misc
-
- [ ] Work on `about:blank`
- [ ] Able to map `<A-Z>` key.
- [ ] Open file menu by <kbd>Alt</kbd>+<kbd>F</kbd> (Other than Mac OS)
--- a/e2e/completion_open.test.js
+++ b/e2e/completion_open.test.js
@ -44,7 +44,7 @@ describe("completion on open/tabopen/winopen commands", () => {
    });
    
    // Add item into hitories
-    await session.navigateTo(`https://i-beam.org`);
+    await session.navigateTo(`https://i-beam.org/404`);
  });

  after(async() => {
--- a/e2e/options.test.js
+++ b/e2e/options.test.js
@ -0,0 +1,99 @@
+const express = require('express');
+const lanthan = require('lanthan');
+const path = require('path');
+const assert = require('assert');
+const eventually = require('./eventually');
+
+const newApp = () => {
+  let app = express();
+  app.get('/', (req, res) => {
+    res.send(`<!DOCTYPEhtml>
+<html lang="en">
+  <body style="width:10000px; height:10000px"></body>
+</html">`);
+  });
+  return app;
+};
+
+describe("options page", () => {
+  const port = 12321;
+  let http;
+  let firefox;
+  let session;
+  let browser;
+
+  before(async() => {
+    http = newApp().listen(port);
+
+    firefox = await lanthan.firefox({
+      spy: path.join(__dirname, '..'),
+      builderf: (builder) => {
+        builder.addFile('build/settings.js');
+        builder.addFile('build/settings.html');
+      },
+    });
+    await firefox.session.installAddonFromPath(path.join(__dirname, '..'));
+    session = firefox.session;
+    browser = firefox.browser;
+  });
+
+  after(async() => {
+    if (firefox) {
+      await firefox.close();
+    }
+
+    http.close();
+  });
+
+  beforeEach(async() => {
+    let tabs = await browser.tabs.query({});
+    for (let tab of tabs.slice(1)) {
+      await browser.tabs.remove(tab.id);
+    }
+  })
+
+  const updateTextarea = async(value) => {
+    let textarea = await session.findElementByCSS('textarea');
+    await session.executeScript(`document.querySelector('textarea').value = '${value}'`)
+    await textarea.sendKeys(' ');
+    await session.executeScript(() => document.querySelector('textarea').blur());
+  }
+
+  it('saves current config on blur', async () => {
+    let url = await browser.runtime.getURL("build/settings.html")
+    await session.navigateTo(url);
+
+    await updateTextarea(`{ "blacklist": [ "https://example.com" ] }`);
+
+    let { settings } = await browser.storage.local.get('settings');
+    assert.equal(settings.source, 'json')
+    assert.equal(settings.json, '{ "blacklist": [ "https://example.com" ] } ')
+
+    await updateTextarea(`invalid json`);
+
+    settings = (await browser.storage.local.get('settings')).settings;
+    assert.equal(settings.source, 'json')
+    assert.equal(settings.json, '{ "blacklist": [ "https://example.com" ] } ')
+
+    let error = await session.findElementByCSS('.settings-ui-input-error');
+    let text = await error.getText();
+    assert.ok(text.startsWith('SyntaxError:'))
+  });
+
+  it('updates keymaps without reloading', async () => {
+    await browser.tabs.create({ url: `http://127.0.0.1:${port}`, active: false });
+    let url = await browser.runtime.getURL("build/settings.html")
+    await session.navigateTo(url);
+
+    let handles = await session.getWindowHandles();
+    await updateTextarea(`{ "keymaps": { "zz": { "type": "scroll.vertically", "count": 10 } } }`);
+
+    await session.switchToWindow(handles[1]);
+
+    let body = await session.findElementByCSS('body');
+    await body.sendKeys('zz')
+
+    let y = await session.executeScript(() => window.pageYOffset);
+    assert.equal(y, 640);
+  })
+});
--- a/e2e/options_form.test.js
+++ b/e2e/options_form.test.js
@ -0,0 +1,125 @@
+const lanthan = require('lanthan');
+const path = require('path');
+const assert = require('assert');
+
+describe("options form page", () => {
+  let firefox;
+  let session;
+  let browser;
+
+  beforeEach(async() => {
+    firefox = await lanthan.firefox({
+      spy: path.join(__dirname, '..'),
+      builderf: (builder) => {
+        builder.addFile('build/settings.js');
+        builder.addFile('build/settings.html');
+      },
+    });
+    await firefox.session.installAddonFromPath(path.join(__dirname, '..'));
+    session = firefox.session;
+    browser = firefox.browser;
+
+    let tabs = await browser.tabs.query({});
+    for (let tab of tabs.slice(1)) {
+      await browser.tabs.remove(tab.id);
+    }
+  })
+
+  afterEach(async() => {
+    if (firefox) {
+      await firefox.close();
+    }
+  })
+
+  const setBlacklistValue = async(nth, value) => {
+    let selector = '.form-blacklist-form .column-url';
+    let input = (await session.findElementsByCSS(selector))[nth];
+    await input.sendKeys(value);
+    await session.executeScript(`document.querySelectorAll('${selector}')[${nth}].blur()`);
+  }
+
+  const setSearchEngineValue = async(nth, name, url) => {
+    let selector = '.form-search-form input.column-name';
+    let input = (await session.findElementsByCSS(selector))[nth];
+    await input.sendKeys(name);
+    await session.executeScript(`document.querySelectorAll('${selector}')[${nth}].blur()`);
+
+    selector = '.form-search-form input.column-url';
+    input = (await session.findElementsByCSS(selector))[nth];
+    await input.sendKeys(url);
+    await session.executeScript(`document.querySelectorAll('${selector}')[${nth}].blur()`);
+  }
+
+  it('switch to form settings', async () => {
+    let url = await browser.runtime.getURL("build/settings.html")
+    await session.navigateTo(url);
+
+    let useFormInput = await session.findElementByCSS('#setting-source-form');
+    await useFormInput.click();
+    await session.acceptAlert();
+
+    let { settings } = await browser.storage.local.get('settings');
+    assert.equal(settings.source, 'form')
+  })
+
+  it('add blacklist', async () => {
+    let url = await browser.runtime.getURL("build/settings.html")
+    await session.navigateTo(url);
+
+    let useFormInput = await session.findElementByCSS('#setting-source-form');
+    await useFormInput.click();
+    await session.acceptAlert();
+    await session.executeScript(() => window.scrollBy(0, 1000));
+
+    // assert default
+    let settings = (await browser.storage.local.get('settings')).settings;
+    assert.deepEqual(settings.form.blacklist, [])
+
+    // add blacklist items
+    let addButton = await session.findElementByCSS('.form-blacklist-form .ui-add-button');
+    await addButton.click();
+    await setBlacklistValue(0, 'google.com')
+
+    settings = (await browser.storage.local.get('settings')).settings;
+    assert.deepEqual(settings.form.blacklist, ['google.com'])
+
+    await addButton.click();
+    await setBlacklistValue(1, 'yahoo.com')
+
+    settings = (await browser.storage.local.get('settings')).settings;
+    assert.deepEqual(settings.form.blacklist, ['google.com', 'yahoo.com'])
+
+    // delete first item
+    let deleteButton = (await session.findElementsByCSS('.form-blacklist-form .ui-delete-button'))[0];
+    await deleteButton.click()
+
+    settings = (await browser.storage.local.get('settings')).settings;
+    assert.deepEqual(settings.form.blacklist, ['yahoo.com'])
+  });
+
+  it('add search engines', async () => {
+    let url = await browser.runtime.getURL("build/settings.html")
+    await session.navigateTo(url);
+
+    let useFormInput = await session.findElementByCSS('#setting-source-form');
+    await useFormInput.click();
+    await session.acceptAlert();
+
+    // assert default
+    let settings = (await browser.storage.local.get('settings')).settings;
+    assert.deepEqual(settings.form.search.default, 'google');
+
+    // change default
+    let radio = (await session.findElementsByCSS('.form-search-form input[type=radio]'))[2];
+    await radio.click();
+    settings = (await browser.storage.local.get('settings')).settings;
+    assert.deepEqual(settings.form.search.default, 'bing');
+
+    let addButton = await session.findElementByCSS('.form-search-form .ui-add-button');
+    await addButton.click();
+    await setSearchEngineValue(6, 'yippy', 'https://www.yippy.com/search?query={}');
+
+    settings = (await browser.storage.local.get('settings')).settings;
+    assert.deepEqual(settings.form.search.engines[6], ['yippy', 'https://www.yippy.com/search?query={}']);
+  });
+});
--- a/package-lock.json
+++ b/package-lock.json
--- a/package.json
+++ b/package.json
@ -3,12 +3,12 @@
  "description": "Vim vixen",
  "scripts": {
    "start": "webpack --mode development -w --debug --devtool inline-source-map",
-    "build": "NODE_ENV=production webpack --mode production --progress --display-error-details",
+    "build": "NODE_ENV=production webpack --mode production --progress --display-error-details --devtool inline-source-map",
    "package": "npm run build && script/package",
    "lint": "eslint --ext .js,.jsx,.ts,.tsx src",
    "type-checks": "tsc --noEmit",
    "test": "karma start",
-    "test:e2e": "mocha --timeout 8000 --retries 5 e2e"
+    "test:e2e": "mocha --timeout 10000 --retries 5 e2e"
  },
  "repository": {
    "type": "git",
@ -21,49 +21,50 @@
  },
  "homepage": "https://github.com/ueokande/vim-vixen",
  "devDependencies": {
-    "@types/chai": "^4.1.7",
-    "@types/mocha": "^5.2.6",
+    "@types/chai": "^4.2.0",
+    "@types/mocha": "^5.2.7",
    "@types/prop-types": "^15.7.1",
-    "@types/react": "^16.8.18",
-    "@types/react-dom": "^16.8.4",
-    "@types/react-redux": "^7.0.9",
+    "@types/react": "^16.9.2",
+    "@types/react-dom": "^16.9.0",
+    "@types/react-redux": "^7.1.2",
    "@types/redux-promise": "^0.5.28",
-    "@types/sinon": "^7.0.11",
-    "@typescript-eslint/eslint-plugin": "^1.9.0",
+    "@types/sinon": "^7.0.13",
+    "@typescript-eslint/eslint-plugin": "^2.0.0",
+    "@typescript-eslint/parser": "^2.0.0",
    "chai": "^4.2.0",
-    "css-loader": "^2.1.1",
-    "eslint": "^5.16.0",
-    "eslint-plugin-react": "^7.13.0",
+    "css-loader": "^3.2.0",
+    "eslint": "^6.2.2",
+    "eslint-plugin-react": "^7.14.3",
    "html-webpack-plugin": "^3.2.0",
-    "jszip": "^3.2.1",
-    "karma": "^4.1.0",
-    "karma-firefox-launcher": "^1.1.0",
+    "jszip": "^3.2.2",
+    "karma": "^4.2.0",
+    "karma-firefox-launcher": "^1.2.0",
    "karma-html2js-preprocessor": "^1.1.0",
    "karma-mocha": "^1.3.0",
    "karma-mocha-reporter": "^2.2.5",
    "karma-sinon": "^1.0.5",
    "karma-sourcemap-loader": "^0.3.7",
-    "karma-webpack": "^3.0.5",
+    "karma-webpack": "^4.0.2",
    "lanthan": "git+https://github.com/ueokande/lanthan.git#master",
-    "mocha": "^6.1.4",
+    "mocha": "^6.2.0",
    "node-sass": "^4.12.0",
-    "react": "^16.8.6",
-    "react-dom": "^16.8.6",
-    "react-redux": "^7.0.3",
-    "react-test-renderer": "^16.8.6",
-    "redux": "^4.0.1",
+    "react": "^16.9.0",
+    "react-dom": "^16.9.0",
+    "react-redux": "^7.1.1",
+    "react-test-renderer": "^16.9.0",
+    "redux": "^4.0.4",
    "redux-promise": "^0.6.0",
    "reflect-metadata": "^0.1.13",
-    "sass-loader": "^7.1.0",
-    "sinon": "^7.3.2",
+    "sass-loader": "^7.3.1",
+    "sinon": "^7.4.1",
    "sinon-chrome": "^3.0.1",
-    "style-loader": "^0.23.1",
-    "ts-loader": "^6.0.1",
-    "tsyringe": "^3.2.0",
-    "typescript": "^3.4.5",
-    "web-ext-types": "^3.1.0",
+    "style-loader": "^1.0.0",
+    "ts-loader": "^6.0.4",
+    "tsyringe": "^3.3.0",
+    "typescript": "^3.6.2",
+    "web-ext-types": "^3.2.1",
    "webextensions-api-fake": "^0.8.0",
-    "webpack": "^4.32.2",
-    "webpack-cli": "^3.3.2"
+    "webpack": "^4.39.2",
+    "webpack-cli": "^3.3.7"
  }
 }
--- a/src/background/infrastructures/ContentMessageClient.ts
+++ b/src/background/infrastructures/ContentMessageClient.ts
@ -22,14 +22,14 @@ export default class ContentMessageClient {
    return enabled as any as boolean;
  }

-  toggleAddonEnabled(tabId: number): Promise<void> {
-    return browser.tabs.sendMessage(tabId, {
+  async toggleAddonEnabled(tabId: number): Promise<void> {
+    await browser.tabs.sendMessage(tabId, {
      type: messages.ADDON_TOGGLE_ENABLED,
    });
  }

-  scrollTo(tabId: number, x: number, y: number): Promise<void> {
-    return browser.tabs.sendMessage(tabId, {
+  async scrollTo(tabId: number, x: number, y: number): Promise<void> {
+    await browser.tabs.sendMessage(tabId, {
      type: messages.TAB_SCROLL_TO,
      x,
      y,
--- a/src/background/presenters/NotifyPresenter.ts
+++ b/src/background/presenters/NotifyPresenter.ts
@ -26,11 +26,20 @@ export default class NotifyPresenter {
    });
  }

-  async notifyInvalidSettings(): Promise<void> {
+  async notifyInvalidSettings(onclick: () => void): Promise<void> {
    let title = `Loaded settings is invalid`;
    // eslint-disable-next-line max-len
    let message = 'The default settings is used due to the last saved settings is invalid.  Check your current settings from the add-on preference';

+    const listener = (id: string) => {
+      if (id !== NOTIFICATION_ID_INVALID_SETTINGS) {
+        return;
+      }
+      onclick();
+      browser.notifications.onClicked.removeListener(listener);
+    };
+    browser.notifications.onClicked.addListener(listener);
+
    await browser.notifications.create(NOTIFICATION_ID_INVALID_SETTINGS, {
      'type': 'basic',
      'iconUrl': browser.extension.getURL('resources/icon_48x48.png'),
--- a/src/background/usecases/SettingUseCase.ts
+++ b/src/background/usecases/SettingUseCase.ts
@ -21,7 +21,12 @@ export default class SettingUseCase {
  }

  async reload(): Promise<Settings> {
-    let data = await this.persistentSettingRepository.load();
+    let data;
+    try {
+      data = await this.persistentSettingRepository.load();
+    } catch (e) {
+      this.showUnableToLoad(e);
+    }
    if (!data) {
      data = DefaultSettingData;
    }
@ -30,10 +35,17 @@ export default class SettingUseCase {
    try {
      value = data.toSettings();
    } catch (e) {
-      this.notifyPresenter.notifyInvalidSettings();
+      this.showUnableToLoad(e);
      value = DefaultSettingData.toSettings();
    }
    this.settingRepository.update(value!!);
    return value;
  }
+
+  private showUnableToLoad(e: Error) {
+    console.error('unable to load settings', e);
+    this.notifyPresenter.notifyInvalidSettings(() => {
+      browser.runtime.openOptionsPage();
+    });
+  }
 }
--- a/src/settings/components/form/PropertiesForm.tsx
+++ b/src/settings/components/form/PropertiesForm.tsx
@ -25,12 +25,19 @@ class PropertiesForm extends React.Component<Props> {
        Object.keys(types).map((name) => {
          let type = types[name];
          let inputType = '';
+          let onChange = this.bindValue.bind(this);
          if (type === 'string') {
            inputType = 'text';
          } else if (type === 'number') {
            inputType = 'number';
          } else if (type === 'boolean') {
            inputType = 'checkbox';
+
+            // Settings are saved onBlur, but checkbox does not fire it
+            onChange = (e) => {
+              this.bindValue(e);
+              this.props.onBlur();
+            };
          } else {
            return null;
          }
@ -40,7 +47,7 @@ class PropertiesForm extends React.Component<Props> {
              <input type={inputType} name={name}
                className='column-input'
                value={value[name] ? value[name] : ''}
-                onChange={this.bindValue.bind(this)}
+                onChange={onChange}
                onBlur={this.props.onBlur}
                checked={value[name]}
              />
--- a/src/settings/storage.ts
+++ b/src/settings/storage.ts
@ -5,7 +5,12 @@ export const load = async(): Promise<SettingData> => {
  if (!settings) {
    return DefaultSettingData;
  }
+  try {
    return SettingData.valueOf(settings as any);
+  } catch (e) {
+    console.error('unable to load settings', e);
+    return DefaultSettingData;
+  }
 };

 export const save = (data: SettingData) => {
--- a/src/shared/Settings.ts
+++ b/src/shared/Settings.ts
@ -21,13 +21,6 @@ export default interface Settings {
  blacklist: string[];
 }

-const DefaultProperties: Properties = PropertyDefs.defs.reduce(
-  (o: {[name: string]: PropertyDefs.Type}, def) => {
-    o[def.name] = def.defaultValue;
-    return o;
-  }, {}) as Properties;
-
-
 export const keymapsValueOf = (o: any): Keymaps => {
  return Object.keys(o).reduce((keymaps: Keymaps, key: string): Keymaps => {
    let op = operations.valueOf(o[key]);
@ -82,7 +75,7 @@ export const propertiesValueOf = (o: any): Properties => {
    }
  }
  return {
-    ...DefaultProperties,
+    ...PropertyDefs.defaultValues,
    ...o,
  };
 };
--- a/src/shared/operations.ts
+++ b/src/shared/operations.ts
@ -368,7 +368,9 @@ export type Operation =
 const assertOptionalBoolean = (obj: any, name: string) => {
  if (Object.prototype.hasOwnProperty.call(obj, name) &&
      typeof obj[name] !== 'boolean') {
-    throw new TypeError(`Not a boolean parameter: '${name}'`);
+    throw new TypeError(
+      `Not a boolean parameter: '${name} (${typeof obj[name]})'`,
+    );
  }
 };

@ -376,7 +378,9 @@ const assertOptionalString = (obj: any, name: string, values?: string[]) => {
  if (Object.prototype.hasOwnProperty.call(obj, name)) {
    let value = obj[name];
    if (typeof value !== 'string') {
-      throw new TypeError(`Not a string parameter: '${name}'`);
+      throw new TypeError(
+        `Not a string parameter: '${name}' (${typeof value})`,
+      );
    }
    if (values && values.length && values.indexOf(value) === -1) {
      // eslint-disable-next-line max-len
@ -421,32 +425,32 @@ export const valueOf = (o: any): Operation => {
    assertOptionalBoolean(o, 'background');
    return {
      type: FOLLOW_START,
-      newTab: Boolean(typeof o.newTab === undefined ? false : o.newTab),
-      background: Boolean(typeof o.background === undefined ? true : o.background), // eslint-disable-line max-len
+      newTab: Boolean(typeof o.newTab === 'undefined' ? false : o.newTab),
+      background: Boolean(typeof o.background === 'undefined' ? true : o.background), // eslint-disable-line max-len
    };
  case PAGE_HOME:
    assertOptionalBoolean(o, 'newTab');
    return {
      type: PAGE_HOME,
-      newTab: Boolean(typeof o.newTab === undefined ? false : o.newTab),
+      newTab: Boolean(typeof o.newTab === 'undefined' ? false : o.newTab),
    };
  case TAB_CLOSE:
    assertOptionalString(o, 'select', ['left', 'right']);
    return {
      type: TAB_CLOSE,
-      select: (typeof o.select === undefined ? 'right' : o.select),
+      select: (typeof o.select === 'undefined' ? 'right' : o.select),
    };
  case TAB_RELOAD:
    assertOptionalBoolean(o, 'cache');
    return {
      type: TAB_RELOAD,
-      cache: Boolean(typeof o.cache === undefined ? false : o.cache),
+      cache: Boolean(typeof o.cache === 'undefined' ? false : o.cache),
    };
  case URLS_PASTE:
    assertOptionalBoolean(o, 'newTab');
    return {
      type: URLS_PASTE,
-      newTab: Boolean(typeof o.newTab === undefined ? false : o.newTab),
+      newTab: Boolean(typeof o.newTab === 'undefined' ? false : o.newTab),
    };
  case INTERNAL_OPEN_URL:
    assertOptionalBoolean(o, 'newTab');
@ -456,9 +460,9 @@ export const valueOf = (o: any): Operation => {
    return {
      type: INTERNAL_OPEN_URL,
      url: o.url,
-      newTab: Boolean(typeof o.newTab === undefined ? false : o.newTab),
-      newWindow: Boolean(typeof o.newWindow === undefined ? false : o.newWindow), // eslint-disable-line max-len
-      background: Boolean(typeof o.background === undefined ? true : o.background), // eslint-disable-line max-len
+      newTab: Boolean(typeof o.newTab === 'undefined' ? false : o.newTab),
+      newWindow: Boolean(typeof o.newWindow === 'undefined' ? false : o.newWindow), // eslint-disable-line max-len
+      background: Boolean(typeof o.background === 'undefined' ? true : o.background), // eslint-disable-line max-len
    };
  case CANCEL:
  case ADDON_ENABLE:
--- a/src/shared/property-defs.ts
+++ b/src/shared/property-defs.ts
@ -48,3 +48,9 @@ export const defs: Def[] = [
    'which are completed at the open page',
    'sbh'),
 ];
+
+export const defaultValues = {
+  hintchars: 'abcdefghijklmnopqrstuvwxyz',
+  smoothscroll: false,
+  complete: 'sbh',
+};