From c753194941b3c7a6df8ff328fa36b71c854bc26a Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 14:03:48 +0100 Subject: Add Settings.isLoaded to the unified settings implementation --- lib/settings.coffee | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index dd667dbd..2dd6722b 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -30,6 +30,7 @@ Sync = unless chrome.runtime.lastError for own key, value of items Settings.storeAndPropagate key, value if @shouldSyncKey key + Settings.isLoaded = true # Asynchronous message from synced storage. handleStorageUpdate: (changes, area) -> @@ -58,15 +59,20 @@ Sync = if Utils.isExtensionPage() if Utils.isBackgroundPage() settingsCache = localStorage + isPreloaded = true else settingsCache = extend {}, localStorage # Make a copy of the cached settings from localStorage + isPreloaded = true else settingsCache = {} + isPreloaded = false root.Settings = Settings = + isLoaded: isPreloaded cache: settingsCache init: -> Sync.init() get: (key) -> + console.log "WARNING: Settings have not loaded yet; using the default value for #{key}." unless @isLoaded if (key of @cache) then JSON.parse(@cache[key]) else @defaults[key] set: (key, value) -> -- cgit v1.2.3 From 257a219fdfd33c49b565a93dff9d785824533d2a Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 14:25:42 +0100 Subject: Add event listeners to settings, support load events --- lib/settings.coffee | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index 2dd6722b..b5021225 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -31,6 +31,8 @@ Sync = for own key, value of items Settings.storeAndPropagate key, value if @shouldSyncKey key Settings.isLoaded = true + unless isPreloaded + listener() while listener = Settings.eventListeners.load?.pop() # Asynchronous message from synced storage. handleStorageUpdate: (changes, area) -> @@ -70,7 +72,13 @@ else root.Settings = Settings = isLoaded: isPreloaded cache: settingsCache - init: -> Sync.init() + eventListeners: {} + + init: -> + Sync.init() + if isPreloaded + listener() while listener = Settings.eventListeners.load?.pop() + get: (key) -> console.log "WARNING: Settings have not loaded yet; using the default value for #{key}." unless @isLoaded if (key of @cache) then JSON.parse(@cache[key]) else @defaults[key] @@ -91,6 +99,9 @@ root.Settings = Settings = has: (key) -> key of @cache + addEventListener: (eventName, callback) -> + (@eventListeners[eventName] ||= []).push callback + # For settings which require action when their value changes, add hooks to this object, to be called from # options/options.coffee (when the options page is saved), and by Settings.storeAndPropagate (when an # update propagates from chrome.storage.sync). -- cgit v1.2.3 From 0de6b076271b673d0e1dcc2b74b2ddd1646bf08e Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 16:35:09 +0100 Subject: Rewrite settings as a tight wrapper around Settings, tweaks for tests --- lib/settings.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index b5021225..88e3b883 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -81,7 +81,7 @@ root.Settings = Settings = get: (key) -> console.log "WARNING: Settings have not loaded yet; using the default value for #{key}." unless @isLoaded - if (key of @cache) then JSON.parse(@cache[key]) else @defaults[key] + if key of @cache and @cache[key]? then JSON.parse(@cache[key]) else @defaults[key] set: (key, value) -> # Don't store the value if it is equal to the default, so we can change the defaults in the future -- cgit v1.2.3 From 1e380ff702b09c0ba98f8b067b47a02fe7561729 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 18:59:51 +0100 Subject: Add a default value for helpDialog_showAdvancedCommands We need this because Settings rejects key/value pairs from chrome.storage for which there were no default values. Previously, this only meant that the setting would not sync; now it meant that the setting wasn't ever made available to the frontend. This commit fixes it, and now the setting will sync. --- lib/settings.coffee | 1 + 1 file changed, 1 insertion(+) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index 88e3b883..5bbc9719 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -198,6 +198,7 @@ root.Settings = Settings = grabBackFocus: false settingsVersion: Utils.getCurrentVersion() + helpDialog_showAdvancedCommands: false # Export Sync via Settings for tests. root.Settings.Sync = Sync -- cgit v1.2.3 From c62ffa33ad5230b89f44cb8f3268e6a4e48afd52 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 1 Jun 2015 07:04:43 +0100 Subject: Re-work unified settings. This is a minor re-working of #1705 from @mrmr1993. The main changes are: - Simplify initialisation logic. - Always initialise Settings immediately and automatically (ie. don't initialise Settings separately and manually in the background, content scripts, options and tests). - Get rid of addEventListener (it's only being used for on "load"). - Add Settings.use() in its place. --- lib/settings.coffee | 74 +++++++++++++++++++++++------------------------------ 1 file changed, 32 insertions(+), 42 deletions(-) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index 5bbc9719..87fefd95 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -2,8 +2,6 @@ # * Sync.set() and Sync.clear() propagate local changes to chrome.storage.sync. # * Sync.handleStorageUpdate() listens for changes to chrome.storage.sync and propagates those # changes to localStorage and into vimium's internal state. -# * Sync.fetchAsync() polls chrome.storage.sync at startup, similarly propagating -# changes to localStorage and into vimium's internal state. # # The effect is best-effort synchronization of vimium options/settings between # chrome/vimium instances. @@ -13,26 +11,19 @@ # they're always non-empty strings. # -root = exports ? window Sync = - storage: chrome.storage.sync doNotSync: ["settingsVersion", "previousVersion"] - # This is called in main.coffee. - init: -> - chrome.storage.onChanged.addListener (changes, area) -> Sync.handleStorageUpdate changes, area - @fetchAsync() - - # Asynchronous fetch from synced storage, called only at startup. - fetchAsync: -> + init: (onReady) -> + chrome.storage.onChanged.addListener (changes, area) => @handleStorageUpdate changes, area @storage.get null, (items) => unless chrome.runtime.lastError for own key, value of items Settings.storeAndPropagate key, value if @shouldSyncKey key - Settings.isLoaded = true - unless isPreloaded - listener() while listener = Settings.eventListeners.load?.pop() + # We call onReady() even if @storage.get() fails; otherwise, the initialization of Settings never + # completes. + onReady?() # Asynchronous message from synced storage. handleStorageUpdate: (changes, area) -> @@ -53,31 +44,24 @@ Sync = # Should we synchronize this key? shouldSyncKey: (key) -> key not in @doNotSync -# -# Used by all parts of Vimium to manipulate localStorage. -# - -# Select the object to use as the cache for settings. -if Utils.isExtensionPage() - if Utils.isBackgroundPage() - settingsCache = localStorage - isPreloaded = true - else - settingsCache = extend {}, localStorage # Make a copy of the cached settings from localStorage - isPreloaded = true -else - settingsCache = {} - isPreloaded = false - -root.Settings = Settings = - isLoaded: isPreloaded - cache: settingsCache - eventListeners: {} +Settings = + isLoaded: false + cache: {} + onLoadedListeners: [] init: -> - Sync.init() - if isPreloaded - listener() while listener = Settings.eventListeners.load?.pop() + # On extension pages, we use localStorage (or a copy of it) as the cache. + if Utils.isExtensionPage() + @cache = if Utils.isBackgroundPage() then localStorage else extend {}, localStorage + @postInit() + + Sync.init => @postInit() + + postInit: -> + wasLoaded = @isLoaded + @isLoaded = true + unless wasLoaded + listener() while listener = @onLoadedListeners.pop() get: (key) -> console.log "WARNING: Settings have not loaded yet; using the default value for #{key}." unless @isLoaded @@ -99,8 +83,9 @@ root.Settings = Settings = has: (key) -> key of @cache - addEventListener: (eventName, callback) -> - (@eventListeners[eventName] ||= []).push callback + use: (key, callback) -> + callCallback = => callback @get key + if @isLoaded then callCallback() else @onLoadedListeners.push => callCallback # For settings which require action when their value changes, add hooks to this object, to be called from # options/options.coffee (when the options page is saved), and by Settings.storeAndPropagate (when an @@ -111,7 +96,7 @@ root.Settings = Settings = performPostUpdateHook: (key, value) -> @postUpdateHooks[key]? value - # Only ever called from asynchronous synced-storage callbacks (fetchAsync and handleStorageUpdate). + # Only ever called from asynchronous synced-storage callbacks (on start up and handleStorageUpdate). storeAndPropagate: (key, value) -> return unless key of @defaults return if value and key of @cache and @cache[key] is value @@ -200,8 +185,7 @@ root.Settings = Settings = settingsVersion: Utils.getCurrentVersion() helpDialog_showAdvancedCommands: false -# Export Sync via Settings for tests. -root.Settings.Sync = Sync +Settings.init() # Perform migration from old settings versions, if this is the background page. if Utils.isBackgroundPage() @@ -218,3 +202,9 @@ if Utils.isBackgroundPage() unless chrome.runtime.lastError or items.findModeRawQueryList rawQuery = Settings.get "findModeRawQuery" chrome.storage.local.set findModeRawQueryList: (if rawQuery then [ rawQuery ] else []) + +root = exports ? window +root.Settings = Settings + +# Export Sync via Settings for tests. +root.Settings.Sync = Sync -- cgit v1.2.3 From 83fefcae893f9ba57f291681f7b0328e6ee41db0 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 1 Jun 2015 09:52:04 +0100 Subject: Only propagate changes from chrome.storage.sync. --- lib/settings.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index 87fefd95..040c1697 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -27,8 +27,9 @@ Sync = # Asynchronous message from synced storage. handleStorageUpdate: (changes, area) -> - for own key, change of changes - Settings.storeAndPropagate key, change?.newValue if @shouldSyncKey key + if area == "sync" + for own key, change of changes + Settings.storeAndPropagate key, change?.newValue if @shouldSyncKey key # Only called synchronously from within vimium, never on a callback. # No need to propagate updates to the rest of vimium, that's already been done. -- cgit v1.2.3 From 5f0400ebac5867df74225b987ea1238bdaeb40b2 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 1 Jun 2015 10:53:42 +0100 Subject: Refactor and eliminate Sync object. --- lib/settings.coffee | 149 +++++++++++++++++++--------------------------------- 1 file changed, 54 insertions(+), 95 deletions(-) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index 040c1697..ee46c0b1 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -1,121 +1,83 @@ -# -# * Sync.set() and Sync.clear() propagate local changes to chrome.storage.sync. -# * Sync.handleStorageUpdate() listens for changes to chrome.storage.sync and propagates those -# changes to localStorage and into vimium's internal state. -# -# The effect is best-effort synchronization of vimium options/settings between -# chrome/vimium instances. -# -# NOTE: -# Values handled within this module are ALWAYS already JSON.stringifed, so -# they're always non-empty strings. -# - -Sync = - storage: chrome.storage.sync - doNotSync: ["settingsVersion", "previousVersion"] - - init: (onReady) -> - chrome.storage.onChanged.addListener (changes, area) => @handleStorageUpdate changes, area - @storage.get null, (items) => - unless chrome.runtime.lastError - for own key, value of items - Settings.storeAndPropagate key, value if @shouldSyncKey key - # We call onReady() even if @storage.get() fails; otherwise, the initialization of Settings never - # completes. - onReady?() - - # Asynchronous message from synced storage. - handleStorageUpdate: (changes, area) -> - if area == "sync" - for own key, change of changes - Settings.storeAndPropagate key, change?.newValue if @shouldSyncKey key - - # Only called synchronously from within vimium, never on a callback. - # No need to propagate updates to the rest of vimium, that's already been done. - set: (key, value) -> - if @shouldSyncKey key - setting = {}; setting[key] = value - @storage.set setting - - # Only called synchronously from within vimium, never on a callback. - clear: (key) -> - @storage.remove key if @shouldSyncKey key - - # Should we synchronize this key? - shouldSyncKey: (key) -> key not in @doNotSync Settings = - isLoaded: false + storage: chrome.storage.sync cache: {} - onLoadedListeners: [] + isLoaded: false + onLoadedCallbacks: [] init: -> - # On extension pages, we use localStorage (or a copy of it) as the cache. if Utils.isExtensionPage() + # On extension pages, we use localStorage (or a copy of it) as the cache. @cache = if Utils.isBackgroundPage() then localStorage else extend {}, localStorage - @postInit() + @onLoaded() - Sync.init => @postInit() + @storage.get null, (items) => + @propagateChangesFromChromeStorage items unless chrome.runtime.lastError + + chrome.storage.onChanged.addListener (changes, area) => + @propagateChangesFromChromeStorage changes if area == "sync" + + @onLoaded() - postInit: -> - wasLoaded = @isLoaded + # Called after @cache has been initialized. On extension pages, this will be called twice, but that does + # not matter because it's idempotent. + onLoaded: -> @isLoaded = true - unless wasLoaded - listener() while listener = @onLoadedListeners.pop() + callback() while callback = @onLoadedCallbacks.pop() + + shouldSyncKey: (key) -> + (key of @defaults) and key not in [ "settingsVersion", "previousVersion" ] + + propagateChangesFromChromeStorage: (changes) -> + @handleUpdateFromChromeStorage key, change?.newValue for own key, change of changes + + handleUpdateFromChromeStorage: (key, value) -> + # Note: value here is either null or a JSONified string. Therefore, even falsy settings values (like + # false, 0 or "") are truthy here. Only null is falsy. + if @shouldSyncKey key + unless value and key of @cache and @cache[key] == value + defaultValue = @defaults[key] + defaultValueJSON = JSON.stringify defaultValue + + if value and value != defaultValueJSON + # Key/value has been changed to a non-default value. + @cache[key] = value + @performPostUpdateHook key, JSON.parse value + else + # The key has been reset to its default value. + delete @cache[key] if key of @cache + @performPostUpdateHook key, defaultValue get: (key) -> console.log "WARNING: Settings have not loaded yet; using the default value for #{key}." unless @isLoaded - if key of @cache and @cache[key]? then JSON.parse(@cache[key]) else @defaults[key] + if key of @cache and @cache[key]? then JSON.parse @cache[key] else @defaults[key] set: (key, value) -> - # Don't store the value if it is equal to the default, so we can change the defaults in the future - if (value == @defaults[key]) - @clear(key) + # Don't store the value if it is equal to the default, so we can change the defaults in the future. + if value == @defaults[key] + @clear key else jsonValue = JSON.stringify value @cache[key] = jsonValue - Sync.set key, jsonValue + if @shouldSyncKey key + setting = {}; setting[key] = jsonValue + @storage.set setting clear: (key) -> - if @has key - delete @cache[key] - Sync.clear key + delete @cache[key] if @has key + @storage.remove key if @shouldSyncKey key has: (key) -> key of @cache use: (key, callback) -> - callCallback = => callback @get key - if @isLoaded then callCallback() else @onLoadedListeners.push => callCallback + invokeCallback = => callback @get key + if @isLoaded then invokeCallback() else @onLoadedCallbacks.push invokeCallback - # For settings which require action when their value changes, add hooks to this object, to be called from - # options/options.coffee (when the options page is saved), and by Settings.storeAndPropagate (when an - # update propagates from chrome.storage.sync). + # For settings which require action when their value changes, add hooks to this object. postUpdateHooks: {} + performPostUpdateHook: (key, value) -> @postUpdateHooks[key]? value - # postUpdateHooks convenience wrapper - performPostUpdateHook: (key, value) -> - @postUpdateHooks[key]? value - - # Only ever called from asynchronous synced-storage callbacks (on start up and handleStorageUpdate). - storeAndPropagate: (key, value) -> - return unless key of @defaults - return if value and key of @cache and @cache[key] is value - defaultValue = @defaults[key] - defaultValueJSON = JSON.stringify(defaultValue) - - if value and value != defaultValueJSON - # Key/value has been changed to non-default value at remote instance. - @cache[key] = value - @performPostUpdateHook key, JSON.parse(value) - else - # Key has been reset to default value at remote instance. - if key of @cache - delete @cache[key] - @performPostUpdateHook key, defaultValue - - # options.coffee and options.html only handle booleans and strings; therefore all defaults must be booleans - # or strings + # Default values for all settings. defaults: scrollStepSize: 60 smoothScroll: true @@ -147,7 +109,7 @@ Settings = exclusionRules: [ # Disable Vimium on Gmail. - { pattern: "http*://mail.google.com/*", passKeys: "" } + { pattern: "https?://mail.google.com/*", passKeys: "" } ] # NOTE: If a page contains both a single angle-bracket link and a double angle-bracket link, then in @@ -206,6 +168,3 @@ if Utils.isBackgroundPage() root = exports ? window root.Settings = Settings - -# Export Sync via Settings for tests. -root.Settings.Sync = Sync -- cgit v1.2.3 From 971b2fbd6b45ff701ed2dc61fadfb4e7a2f20193 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 1 Jun 2015 12:07:31 +0100 Subject: Fix error reading settings from chrome.storage.sync. --- lib/settings.coffee | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index ee46c0b1..ca41ced7 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -12,7 +12,8 @@ Settings = @onLoaded() @storage.get null, (items) => - @propagateChangesFromChromeStorage items unless chrome.runtime.lastError + unless chrome.runtime.lastError + @handleUpdateFromChromeStorage key, value for own key, value of items chrome.storage.onChanged.addListener (changes, area) => @propagateChangesFromChromeStorage changes if area == "sync" -- cgit v1.2.3 From f50d3add7e6c95a5fc2945e067a52634a19c8a65 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 1 Jun 2015 12:26:16 +0100 Subject: Note bug in settings. (This bug has been around for quite some time. I just noticed it now.) --- lib/settings.coffee | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index ca41ced7..343f782a 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -55,6 +55,8 @@ Settings = set: (key, value) -> # Don't store the value if it is equal to the default, so we can change the defaults in the future. + # FIXME(smblott). This test is broken for exclusionRules (for which it is never true). In this case, we + # need some kind of structural equality (or perhaps comparison of JSONified strings). if value == @defaults[key] @clear key else -- cgit v1.2.3 From bc6bde933f3d1d1ccf8bebb547fe5e83d52164b4 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 1 Jun 2015 12:52:49 +0100 Subject: Simplify searchEngines default value. --- lib/settings.coffee | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index 343f782a..3a89f773 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -127,24 +127,23 @@ Settings = # default/fall back search engine searchUrl: "https://www.google.com/search?q=" # put in an example search engine - searchEngines: [ - "w: http://www.wikipedia.org/w/index.php?title=Special:Search&search=%s Wikipedia" - "" - "# More examples." - "#" - "# (Vimium has built-in completion for these.)" - "#" - "# g: http://www.google.com/search?q=%s Google" - "# l: http://www.google.com/search?q=%s&btnI I'm feeling lucky..." - "# y: http://www.youtube.com/results?search_query=%s Youtube" - "# b: https://www.bing.com/search?q=%s Bing" - "# d: https://duckduckgo.com/?q=%s DuckDuckGo" - "# az: http://www.amazon.com/s/?field-keywords=%s Amazon" - "#" - "# Another example (for Vimium does not have completion)." - "#" - "# m: https://www.google.com/maps/search/%s Google Maps" - ].join "\n" + searchEngines: + """ + w: http://www.wikipedia.org/w/index.php?title=Special:Search&search=%s Wikipedia + + # More examples. + # + # (Vimium supports search completion Wikipedia, as + # above, and for these.) + # + # g: http://www.google.com/search?q=%s Google + # l: http://www.google.com/search?q=%s&btnI I'm feeling lucky... + # y: http://www.youtube.com/results?search_query=%s Youtube + # gm: https://www.google.com/maps?q=%s Google maps + # b: https://www.bing.com/search?q=%s Bing + # d: https://duckduckgo.com/?q=%s DuckDuckGo + # az: http://www.amazon.com/s/?field-keywords=%s Amazon + """ newTabUrl: "chrome://newtab" grabBackFocus: false -- cgit v1.2.3 From 34f0f90debf0050ece9bd847993f281c1e64be59 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Mon, 1 Jun 2015 14:16:43 +0100 Subject: Always call performPostUpdateHook after Setting updates (.set/.clear) --- lib/settings.coffee | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index 3a89f773..4fafa7d3 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -65,10 +65,12 @@ Settings = if @shouldSyncKey key setting = {}; setting[key] = jsonValue @storage.set setting + @performPostUpdateHook key, value clear: (key) -> delete @cache[key] if @has key @storage.remove key if @shouldSyncKey key + @performPostUpdateHook key, @get key has: (key) -> key of @cache -- cgit v1.2.3