fix: extract a common unload sentry (#1117)

* fix: extract a common unload sentry

* fix: always notify unsaved changes

src/common/index.js

@@ -5,9 +5,6 @@ import { noop } from './util';
 export { normalizeKeys } from './object';
 export * from './util';
 
-export function i18n(name, args) {
-  return browser.i18n.getMessage(name, args) || name;
-}
 export const defaultImage = '/public/images/icon128.png';
 
 export function initHooks() {

src/common/router.js

@@ -1,3 +1,6 @@
+import { showConfirmation } from '#/common/ui';
+import { i18n } from './util';
+
 function parse(hash) {
   const [pathname, search = ''] = hash.split('?');
   const query = search.split('&').reduce((res, seq) => {
@@ -19,21 +22,22 @@ export const lastRoute = () => stack[stack.length - 1] || {};
 
 updateRoute();
 
-function updateRoute() {
+function updateRoute(noConfirm) {
   const hash = window.location.hash.slice(1);
-  if (!route.pinned) {
+  if (noConfirm || !route.confirmChange) {
     Object.assign(route, parse(hash));
   } else if (route.hash !== hash) {
     // restore the pinned route
-    setRoute(route.hash);
+    setRoute(route.hash, false, true);
+    route.confirmChange(hash);
   }
 }
 
 // popstate should be the first to ensure hashchange listeners see the correct lastRoute
 window.addEventListener('popstate', () => stack.pop());
-window.addEventListener('hashchange', updateRoute, false);
+window.addEventListener('hashchange', () => updateRoute(), false);
 
-export function setRoute(hash, replace) {
+export function setRoute(hash, replace, noConfirm) {
   let hashString = `${hash}`;
   if (hashString[0] !== '#') hashString = `#${hashString}`;
   if (replace) {
@@ -42,5 +46,30 @@ export function setRoute(hash, replace) {
     stack.push(Object.assign({}, route));
     window.history.pushState('', null, hashString);
   }
-  updateRoute();
+  updateRoute(noConfirm);
+}
+
+export function getUnloadSentry(onConfirm, onCancel) {
+  async function confirmPopState(hash) {
+    try {
+      // popstate cannot be prevented so we pin current `route` and display a confirmation
+      await showConfirmation(i18n('confirmNotSaved'));
+      setRoute(hash, false, true);
+      onConfirm?.();
+    } catch {
+      onCancel?.();
+    }
+  }
+  function toggle(state) {
+    const onOff = `${state ? 'add' : 'remove'}EventListener`;
+    global[onOff]('beforeunload', onUnload);
+    route.confirmChange = state && confirmPopState;
+  }
+  return toggle;
+}
+
+function onUnload(e) {
+  e.preventDefault();
+  // modern browser show their own message text
+  e.returnValue = i18n('confirmNotSaved');
 }

src/common/ui/index.js

@@ -0,0 +1,51 @@
+import Modal from 'vueleton/lib/modal/bundle';
+import { i18n } from '#/common/util';
+import Message from './message';
+
+export function showMessage(message) {
+  const modal = Modal.show(h => h(Message, {
+    props: { message },
+    on: {
+      dismiss() {
+        modal.close();
+        message.onDismiss?.();
+      },
+    },
+  }), {
+    transition: 'in-out',
+  });
+  if (message.buttons) {
+    // TODO: implement proper keyboard navigation, autofocus, and Enter/Esc in Modal module
+    document.querySelector('.vl-modal button').focus();
+  } else {
+    const timer = setInterval(() => {
+      if (!document.querySelector('.vl-modal .modal-content:hover')) {
+        clearInterval(timer);
+        modal.close();
+      }
+    }, message.timeout || 2000);
+  }
+}
+
+/**
+ * @param {string} text - the text to display in the modal
+ * @param {Object} cfg
+ * @param {string | false} [cfg.input=false] if not false, shows a text input with this string
+ * @param {Object} [cfg.ok] additional props for the Ok button
+ * @param {Object} [cfg.cancel] additional props for the Cancel button
+ * @return {Promise<?string>} resolves on Ok to `false` or the entered string, rejects otherwise
+ */
+export function showConfirmation(text, { ok, cancel, input = false } = {}) {
+  return new Promise((resolve, reject) => {
+    showMessage({
+      input,
+      text,
+      buttons: [
+        { text: i18n('buttonOK'), onClick: resolve, ...ok },
+        { text: i18n('buttonCancel'), onClick: reject, ...cancel },
+      ],
+      onBackdropClick: reject,
+      onDismiss: reject, // Esc key
+    });
+  });
+}

src/common/ui/setting-text.vue

@@ -18,6 +18,7 @@
 </template>
 
 <script>
+import { getUnloadSentry } from '#/common/router';
 import { deepEqual, objectGet } from '../object';
 import options from '../options';
 import defaults from '../options-defaults';
@@ -55,13 +56,21 @@ export default {
       }
       return { value, error };
     },
+    isDirty() {
+      return !deepEqual(this.parsedData.value, this.savedValue || '');
+    },
     canSave() {
-      return !this.parsedData.error && !deepEqual(this.parsedData.value, this.savedValue || '');
+      return !this.parsedData.error && this.isDirty;
     },
     canReset() {
       return !deepEqual(this.parsedData.value, this.defaultValue || '');
     },
   },
+  watch: {
+    isDirty(state) {
+      this.toggleUnloadSentry(state);
+    },
+  },
   created() {
     const handle = this.json
       ? (value => JSON.stringify(value, null, '  '))
@@ -72,9 +81,17 @@ export default {
       this.value = handle(val);
     });
     this.defaultValue = objectGet(defaults, this.name);
+    this.toggleUnloadSentry = getUnloadSentry(() => {
+      // Reset to saved value after confirming loss of data.
+      // The component won't be destroyed on tab change, so the changes are actually kept.
+      // Here we reset it to make sure the user loses the changes when leaving the settings tab.
+      // Otherwise the user may be confused about where the changes are after switching back.
+      this.value = handle(this.savedValue);
+    });
   },
   beforeDestroy() {
     this.revoke();
+    this.toggleUnloadSentry(false);
   },
   methods: {
     onChange() {

src/common/util.js

@@ -1,8 +1,14 @@
+import { browser } from '#/common/consts';
+
 // used in an unsafe context so we need to save the original functions
 const perfNow = performance.now.bind(performance);
 const { random, floor } = Math;
 export const { toString: numberToString } = Number.prototype;
 
+export function i18n(name, args) {
+  return browser.i18n.getMessage(name, args) || name;
+}
+
 export function toString(param) {
   if (param == null) return '';
   return `${param}`;

src/options/utils/index.js

@@ -1,56 +1,5 @@
-import Modal from 'vueleton/lib/modal/bundle';
-import { i18n } from '#/common';
 import { route } from '#/common/router';
-import Message from '../views/message';
 
 export const store = {
   route,
 };
-
-export function showMessage(message) {
-  const modal = Modal.show(h => h(Message, {
-    props: { message },
-    on: {
-      dismiss() {
-        modal.close();
-        message.onDismiss?.();
-      },
-    },
-  }), {
-    transition: 'in-out',
-  });
-  if (message.buttons) {
-    // TODO: implement proper keyboard navigation, autofocus, and Enter/Esc in Modal module
-    document.querySelector('.vl-modal button').focus();
-  } else {
-    const timer = setInterval(() => {
-      if (!document.querySelector('.vl-modal .modal-content:hover')) {
-        clearInterval(timer);
-        modal.close();
-      }
-    }, message.timeout || 2000);
-  }
-}
-
-/**
- * @param {string} text - the text to display in the modal
- * @param {Object} cfg
- * @param {string | false} [cfg.input=false] if not false, shows a text input with this string
- * @param {Object} [cfg.ok] additional props for the Ok button
- * @param {Object} [cfg.cancel] additional props for the Cancel button
- * @return {Promise<?string>} resolves on Ok to `false` or the entered string, rejects otherwise
- */
-export function showConfirmation(text, { ok, cancel, input = false } = {}) {
-  return new Promise((resolve, reject) => {
-    showMessage({
-      input,
-      text,
-      buttons: [
-        { text: i18n('buttonOK'), onClick: resolve, ...ok },
-        { text: i18n('buttonCancel'), onClick: reject, ...cancel },
-      ],
-      onBackdropClick: reject,
-      onDismiss: reject, // Esc key
-    });
-  });
-}

src/options/views/edit/index.vue

@@ -58,10 +58,11 @@
 import CodeMirror from 'codemirror';
 import { debounce, i18n, sendCmd } from '#/common';
 import { deepCopy, deepEqual, objectPick } from '#/common/object';
+import { showMessage } from '#/common/ui';
 import VmCode from '#/common/ui/code';
 import options from '#/common/options';
-import { route } from '#/common/router';
-import { store, showConfirmation, showMessage } from '../../utils';
+import { route, getUnloadSentry } from '#/common/router';
+import { store } from '../../utils';
 import VmSettings from './settings';
 import VmValues from './values';
 import VmHelp from './help';
@@ -90,7 +91,6 @@ const toList = text => (
   .filter(Boolean)
 );
 let savedSettings;
-let showingConfirmation;
 
 let shouldSavePositionOnSave;
 const savePosition = () => {
@@ -180,6 +180,9 @@ export default {
   },
   created() {
     this.script = this.initial;
+    this.toggleUnloadSentry = getUnloadSentry(null, () => {
+      this.$refs.code.cm.focus();
+    });
     document.addEventListener('keydown', this.switchPanel);
     if (options.get('editorWindow') && global.history.length === 1) {
       browser.windows?.getCurrent({ populate: true }).then(setupSavePosition);
@@ -264,19 +267,8 @@ export default {
         showMessage({ text: err });
       }
     },
-    async close() {
-      try {
-        if (store.route.pinned) {
-          showingConfirmation = true;
-          await showConfirmation(i18n('confirmNotSaved'));
-          store.route.pinned = false;
-        }
-        this.$emit('close');
-      } catch (e) {
-        this.$refs.code.cm.focus();
-      } finally {
-        showingConfirmation = false;
-      }
+    close() {
+      this.$emit('close');
     },
     saveClose() {
       this.save().then(this.close);
@@ -301,24 +293,9 @@ export default {
       default:
       }
     },
-    toggleUnloadSentry(state) {
-      const onOff = `${state ? 'add' : 'remove'}EventListener`;
-      global[onOff]('beforeunload', this.onUnload);
-      global[onOff]('popstate', this.onUnload);
-      store.route.pinned = state;
-    },
     onChange() {
       this.canSave = this.codeDirty || !deepEqual(this.settings, savedSettings);
     },
-    /** @param {Event} e */
-    onUnload(e) {
-      // modern browser show their own message text
-      e.returnValue = i18n('confirmNotSaved');
-      // popstate cannot be prevented so we pin current `route` and display a confirmation
-      if (e.type === 'popstate' && !showingConfirmation) {
-        this.close();
-      }
-    },
   },
   beforeDestroy() {
     store.title = null;

src/options/views/edit/values.vue

@@ -69,7 +69,7 @@ import { dumpScriptValue, sendCmd } from '#/common';
 import { mapEntry } from '#/common/object';
 import Icon from '#/common/ui/icon';
 import storage from '#/common/storage';
-import { showMessage } from '../../utils';
+import { showMessage } from '#/common/ui';
 
 const PAGE_SIZE = 25;
 const MAX_LENGTH = 1024;

src/options/views/tab-installed.vue

@@ -130,6 +130,7 @@ import {
   i18n, sendCmd, debounce, makePause,
 } from '#/common';
 import options from '#/common/options';
+import { showConfirmation, showMessage } from '#/common/ui';
 import SettingCheck from '#/common/ui/setting-check';
 import hookSetting from '#/common/hook-setting';
 import Icon from '#/common/ui/icon';
@@ -140,7 +141,7 @@ import storage from '#/common/storage';
 import { loadData } from '#/options';
 import ScriptItem from './script-item';
 import Edit from './edit';
-import { store, showConfirmation, showMessage } from '../utils';
+import { store } from '../utils';
 
 const filterOptions = {
   sort: {

src/options/views/tab-settings/vm-blacklist.vue

@@ -11,7 +11,7 @@
 
 <script>
 import { sendCmd } from '#/common';
-import { showMessage } from '#/options/utils';
+import { showMessage } from '#/common/ui';
 import SettingText from '#/common/ui/setting-text';
 
 export default {

src/options/views/tab-settings/vm-css.vue

@@ -7,7 +7,7 @@
 </template>
 
 <script>
-import { showMessage } from '#/options/utils';
+import { showMessage } from '#/common/ui';
 import SettingText from '#/common/ui/setting-text';
 
 export default {

src/options/views/tab-settings/vm-editor.vue

@@ -11,7 +11,7 @@
 
 <script>
 import options from '#/common/options';
-import { showMessage } from '#/options/utils';
+import { showMessage } from '#/common/ui';
 import SettingText from '#/common/ui/setting-text';
 
 export default {

src/options/views/tab-settings/vm-import.vue

@@ -28,7 +28,7 @@ import { ensureArray, i18n, sendCmd } from '#/common';
 import options from '#/common/options';
 import SettingCheck from '#/common/ui/setting-check';
 import loadZipLibrary from '#/common/zip';
-import { showConfirmation, showMessage } from '../../utils';
+import { showConfirmation, showMessage } from '#/common/ui';
 
 const reports = [];
 

src/options/views/tab-settings/vm-template.vue

@@ -6,7 +6,7 @@
 </template>
 
 <script>
-import { showMessage } from '#/options/utils';
+import { showMessage } from '#/common/ui';
 import SettingText from '#/common/ui/setting-text';
 
 export default {