From 6e4a50f69006f1ea9ee76db957cf6edda0506ee7 Mon Sep 17 00:00:00 2001
From: girst <girst@users.noreply.github.com>
Date: Mon, 5 Aug 2019 03:25:43 +0200
Subject: [PATCH] Port settings page to a modern format

Inline-settings have been dropped in http://bugzil.la/1414406. At the
same time, the <setting> XBL binding has been removed. Here, we recreate
the settings page using (X)HTML opening in a new tab.
The XBL converter (bgrins/xbl-analysis@564f6e402a) was helpful in
reconstructing the <setting> element and the new aboutconfig.js has a
good observer example at the end (-> central@4d5f1517ef85).
---
 extension/install.rdf.tmpl   |   2 +
 extension/lib/help.coffee    |   7 +-
 extension/lib/main.coffee    |   2 +-
 extension/lib/options.coffee | 135 +++++++++++++++++++++++------------
 extension/lib/prefs.coffee   |   6 ++
 extension/options.js         |  12 ++++
 extension/options.xhtml      |  12 ++++
 extension/options.xul        |   6 --
 extension/skin/options.css   |  61 ++++++++++++++++
 9 files changed, 187 insertions(+), 56 deletions(-)
 create mode 100644 extension/options.js
 create mode 100644 extension/options.xhtml
 delete mode 100644 extension/options.xul
 create mode 100644 extension/skin/options.css

diff --git a/extension/install.rdf.tmpl b/extension/install.rdf.tmpl
index 81bfaef..38d687d 100644
--- a/extension/install.rdf.tmpl
+++ b/extension/install.rdf.tmpl
@@ -11,6 +11,8 @@
     <em:bootstrap>true</em:bootstrap>
     <em:multiprocessCompatible>true</em:multiprocessCompatible>
     <em:type>2</em:type>
+    <em:optionsType>3</em:optionsType> <!-- OPTIONS_TYPE_TAB -->
+    <em:optionsURL>chrome://vimfx/content/options.xhtml</em:optionsURL>
 
     <!-- Firefox -->
     <em:targetApplication>
diff --git a/extension/lib/help.coffee b/extension/lib/help.coffee
index 6ed0b26..858ac31 100644
--- a/extension/lib/help.coffee
+++ b/extension/lib/help.coffee
@@ -163,10 +163,11 @@ splitSequence = (sequence, specialKeys) ->
 goToCommandSetting = (window, vimfx, command) ->
   vimfx.goToCommand = command
   removeHelp(window)
-  # Randomize URI to force a reload of the Add-ons Manager if it’s already open.
-  uri = "addons://detail/#{vimfx.id}/preferences?#{Math.random()}"
+  uri = "#{ADDON_PATH}/content/options.xhtml"
   utils.nextTick(window, ->
-    window.BrowserOpenAddonsMgr(uri)
+    window.gBrowser.selectedTab = window.gBrowser.addTab(uri, {
+      triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal()
+    })
   )
 
 search = (content, term) ->
diff --git a/extension/lib/main.coffee b/extension/lib/main.coffee
index 1be0631..84f3e77 100644
--- a/extension/lib/main.coffee
+++ b/extension/lib/main.coffee
@@ -33,7 +33,7 @@ module.exports = (data, reason) ->
   vimfx = new VimFx(modes, parsedOptions)
   vimfx.id = data.id
   vimfx.version = data.version
-  AddonManager.getAddonByID(vimfx.id, (info) -> vimfx.info = info)
+  AddonManager.getAddonByID(vimfx.id).then( (info) -> vimfx.info = info )
 
   utils.loadCss("#{ADDON_PATH}/skin/style.css")
 
diff --git a/extension/lib/options.coffee b/extension/lib/options.coffee
index 20d8791..e4e8499 100644
--- a/extension/lib/options.coffee
+++ b/extension/lib/options.coffee
@@ -14,8 +14,8 @@ TYPE_MAP = {
 
 observe = (options) ->
   observer = new Observer(options)
-  utils.observe('addon-options-displayed', observer)
-  utils.observe('addon-options-hidden',    observer)
+  utils.observe('vimfx-options-displayed', observer)
+  utils.observe('vimfx-options-hidden',    observer)
   module.onShutdown(->
     observer.destroy()
   )
@@ -43,17 +43,79 @@ class BaseObserver
   injectSettings: ->
 
   appendSetting: (attributes) ->
-    setting = @document.createElement('setting')
-    utils.setAttributes(setting, attributes)
-    @container.appendChild(setting)
-    return setting
+    outer = @document.createElement('div')
+    outer.classList.add('setting')
+    outer.classList.add('first-row') if attributes['first-row']
+    outer.classList.add(attributes.class)
+    outer.setAttribute('data-pref', attributes.pref)
+    outer.setAttribute('data-type', attributes.type)
+    label = @document.createElement('label')
+    title = @document.createElement('span')
+    title.className = 'title'
+    title.innerText = attributes.title
+    label.appendChild(title)
+    help = @document.createElement('span')
+    help.className = 'desc'
+    help.innerText = attributes.desc or ''
+    input = @document.createElement('input')
+    _this = this
+    switch attributes.type
+      when 'bool'
+        input.type = 'checkbox'
+        input.checked = prefs.root.get(attributes.pref)
+        input.addEventListener('change', ->
+          prefs.root.set(attributes.pref, input.checked)
+        )
+        prefobserver = prefs.root.observe(attributes.pref, ->
+          input.checked = prefs.root.get(attributes.pref)
+        )
+        @document.defaultView.addEventListener('unload', ->
+          prefs?.root.unobserve(attributes.pref, prefobserver)
+        )
+      when 'integer'
+        input.type = 'number'
+        input.value = prefs.root.get(attributes.pref)
+        input.addEventListener('input', ->
+          prefs.root.set(attributes.pref, parseInt(input.value, 10))
+        )
+        prefobserver = prefs.root.observe(attributes.pref, ->
+          input.value = prefs.root.get(attributes.pref)
+        )
+        @document.defaultView.addEventListener('unload', ->
+          prefs?.root.unobserve(attributes.pref, prefobserver)
+        )
+      when 'string'
+        input.type = 'text'
+        input.value = prefs.root.get(attributes.pref)
+        input.addEventListener('input', ->
+          prefs.root.set(attributes.pref, input.value)
+          if outer.classList.contains('is-shortcut')
+            _this.refreshShortcutErrors()
+        )
+        prefobserver = prefs.root.observe(attributes.pref, ->
+          input.value = prefs.root.get(attributes.pref)
+        )
+        @document.defaultView.addEventListener('unload', ->
+          prefs?.root.unobserve(attributes.pref, prefobserver)
+        )
+      when 'control' # injectHeader special case
+        control = @document.createElement('span')
+        control.className = 'control'
+        # can't use <label> when control has multiple buttons:
+        label = @document.createElement('span')
+        label.appendChild(title)
+        label.appendChild(control)
+    label.appendChild(input) if attributes.pref # some nodes are just headlines
+    outer.appendChild(label)
+    outer.appendChild(help) # needed even if empty, as validator puts error here
+    @container.appendChild(outer)
+    return outer
 
   observe: (@document, topic, addonId) ->
-    return unless addonId == @options.id
     switch topic
-      when 'addon-options-displayed'
+      when 'vimfx-options-displayed'
         @init()
-      when 'addon-options-hidden'
+      when 'vimfx-options-hidden'
         @destroy()
 
   init: ->
@@ -73,14 +135,13 @@ class Observer extends BaseObserver
     @injectOptions()
     @injectShortcuts()
     @setupKeybindings()
-    @setupValidation()
 
     if @vimfx.goToCommand
       utils.nextTick(@document.ownerGlobal, =>
         {pref} = @vimfx.goToCommand
-        setting = @container.querySelector("setting[pref='#{pref}']")
+        setting = @container.querySelector("[data-pref='#{pref}']")
         setting.scrollIntoView()
-        setting.input.select()
+        setting.querySelector('input').select()
         @vimfx.goToCommand = null
       )
 
@@ -97,29 +158,28 @@ class Observer extends BaseObserver
       )
       'first-row': 'true'
     })
+    setting.id = 'header'
 
     href = "#{@vimfx.info.homepageURL}/tree/master/documentation#contents"
-    docsLink = @document.createElement('label')
+    docsLink = @document.createElement('a')
+    docsLink.innerText = translate('prefs.documentation')
     utils.setAttributes(docsLink, {
-      value: translate('prefs.documentation')
       href
-      crop: 'end'
-      class: 'text-link'
+      target: '_blank'
     })
-    docsLink.style.MozBoxFlex = '1'
-    setting.appendChild(docsLink)
+    setting.querySelector('.control').appendChild(docsLink)
 
     for key, fn of BUTTONS
       button = @document.createElement('button')
-      button.setAttribute('label', translate("prefs.#{key}.label"))
+      button.innerText = translate("prefs.#{key}.label")
       button.onclick = runWithVim.bind(null, @vimfx, fn)
-      setting.appendChild(button)
+      setting.querySelector('.control').appendChild(button)
 
     return
 
   injectOptions: ->
     for key, value of defaults.options
-      setting = @appendSetting({
+      @appendSetting({
         pref: "#{defaults.BRANCH}#{key}"
         type: @type(value)
         title: translate("pref.#{key}.title")
@@ -165,10 +225,11 @@ class Observer extends BaseObserver
     # element in these listeners!
     quote = false
     @listen(@container, 'keydown', (event) =>
-      setting = event.target
-      isString = (setting.type == 'string')
+      input = event.target
+      isString = (input.type == 'text')
 
-      {input, pref} = setting
+      setting = input.closest('.setting')
+      pref = setting.getAttribute('data-pref')
       keyString = @vimfx.stringifyKeyEvent(event)
 
       # Some shortcuts only make sense for string settings. We still match
@@ -182,6 +243,7 @@ class Observer extends BaseObserver
         when quote
           break unless isString
           utils.insertText(input, keyString)
+          prefs.root.set(pref, input.value)
           quote = false
         when keyString == @vimfx.options['options.key.quote']
           break unless isString
@@ -192,40 +254,21 @@ class Observer extends BaseObserver
         when keyString == @vimfx.options['options.key.insert_default']
           break unless isString
           utils.insertText(input, prefs.root.default.get(pref))
+          prefs.root.set(pref, input.value)
         when keyString == @vimfx.options['options.key.reset_default']
           prefs.root.set(pref, null)
         else
           return
 
       event.preventDefault()
-      setting.valueToPreference()
       @refreshShortcutErrors()
     )
     @listen(@container, 'blur', -> quote = false)
 
-  setupValidation: ->
-    @listen(@container, 'input', (event) =>
-      setting = event.target
-      # Disable default behavior of updating the pref of the setting on each
-      # input. Do it on the 'change' event instead (see below), because all
-      # settings are validated and auto-adjusted as soon as the pref changes.
-      event.stopPropagation()
-      if setting.classList.contains('is-shortcut')
-        # However, for the shortcuts we _do_ want live validation, because they
-        # cannot be auto-adjusted. Instead an error message is shown.
-        setting.valueToPreference()
-        @refreshShortcutErrors()
-    )
-
-    @listen(@container, 'change', (event) ->
-      setting = event.target
-      unless setting.classList.contains('is-shortcut')
-        setting.valueToPreference()
-    )
-
   refreshShortcutErrors: ->
     for setting in @container.getElementsByClassName('is-shortcut')
-      setting.setAttribute('desc', @generateErrorMessage(setting.pref))
+      setting.querySelector('.desc').innerText =
+        @generateErrorMessage(setting.getAttribute('data-pref'))
     return
 
 resetAllPrefs = (vim) ->
diff --git a/extension/lib/prefs.coffee b/extension/lib/prefs.coffee
index 67b14ce..a9b2579 100644
--- a/extension/lib/prefs.coffee
+++ b/extension/lib/prefs.coffee
@@ -65,6 +65,10 @@ observe = (branch, domain, callback) ->
   module.onShutdown(->
     branch.removeObserver(domain, observer)
   )
+  return observer
+
+unobserve = (branch, domain, observer) ->
+  branch.removeObserver(domain, observer)
 
 module.exports = {
   get: get.bind(null, branches.addon.user)
@@ -72,6 +76,7 @@ module.exports = {
   has: has.bind(null, branches.addon.user)
   tmp: tmp.bind(null, branches.addon.user)
   observe: observe.bind(null, branches.addon.user)
+  unobserve: unobserve.bind(null, branches.addon.user)
   default: {
     get: get.bind(null, branches.addon.default)
     set: set.bind(null, branches.addon.default)
@@ -86,6 +91,7 @@ module.exports = {
     has: has.bind(null, branches.root.user)
     tmp: tmp.bind(null, branches.root.user)
     observe: observe.bind(null, branches.root.user)
+    unobserve: unobserve.bind(null, branches.root.user)
     default: {
       get: get.bind(null, branches.root.default)
       set: set.bind(null, branches.root.default)
diff --git a/extension/options.js b/extension/options.js
new file mode 100644
index 0000000..174e420
--- /dev/null
+++ b/extension/options.js
@@ -0,0 +1,12 @@
+"use strict";
+
+const { utils: Cu } = Components;
+const { Services } = Cu.import("resource://gre/modules/Services.jsm", {});
+
+window.addEventListener("load", () => {
+  Services.obs.notifyObservers(document, "vimfx-options-displayed", "");
+}, { once: true });
+
+window.addEventListener("unload", () => {
+  Services.obs.notifyObservers(document, "vimfx-options-hidden", "");
+}, { once: true });
diff --git a/extension/options.xhtml b/extension/options.xhtml
new file mode 100644
index 0000000..cd4710e
--- /dev/null
+++ b/extension/options.xhtml
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml">
+  <head>
+    <title>VimFx</title>
+    <script src="chrome://vimfx/content/options.js"></script>
+    <link rel="stylesheet" href="chrome://vimfx/skin/options.css" />
+    <link rel="stylesheet" href="chrome://global/skin/in-content/common.css" />
+  </head>
+  <body>
+    <div id="detail-rows"></div>
+  </body>
+</html>
diff --git a/extension/options.xul b/extension/options.xul
deleted file mode 100644
index 48cdba6..0000000
--- a/extension/options.xul
+++ /dev/null
@@ -1,6 +0,0 @@
-<!--
-Dummy file to enable inline options. All elements are added by
-lib/options.coffee. Include an empty element so that an 'no element found' error
-is not logged to the error console.
--->
-<vbox />
diff --git a/extension/skin/options.css b/extension/skin/options.css
new file mode 100644
index 0000000..83024dd
--- /dev/null
+++ b/extension/skin/options.css
@@ -0,0 +1,61 @@
+#detail-rows {
+  padding: 0 2em;
+  margin: auto;
+  min-width: 550px;
+}
+.setting {
+  box-sizing: border-box;
+  width: 100%;
+  border-top: 1px solid #c1c1c1;
+  min-height: 35px;
+  line-height: 20px;
+}
+.setting>:first-child {
+  /* usually the <label>, except when it has a .control */
+  display: flex;
+}
+.setting.first-row {
+  border-top: none;
+  margin-top: 2em;
+}
+.setting .title {
+  flex: 1;
+  min-width: 15em;
+  max-width: 25em;
+  padding: 4px;
+}
+.setting .desc {
+  color: graytext;
+  display: block;
+  margin: 4px 0;
+  margin-inline-start: 2em;
+}
+.setting .desc:empty {
+  display: none;
+}
+.setting input[type="checkbox"] {
+  width: 24px;
+  height: 24px;
+  margin: 2px 0;
+}
+.setting input[type="text"],
+.setting input[type="number"] {
+  flex: 1;
+  min-width: 200px;
+  margin: 2px 0;
+  padding: 0.5em !important;
+  background: #fff;
+  border: 1px solid #ccc;
+  border-radius: 4px;
+  color: #333;
+}
+
+#header .control {
+  flex: 1;
+  text-align: right;
+  height: 1px; /* let it overlap the empty part of the description */
+}
+#header .control a {
+  padding-top: 4px; /* match .title */
+  float: left;
+}
-- 
2.39.3