From aa00e29dc2533b6701c65935223599671c5833b1 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 14 Jun 2015 07:40:13 +0100 Subject: Add default value for regexFindMode setting. Because we haven't had a default value for this setting, we never sync it, which means -- when its not at its default value -- it isn't picked up in content scripts by the new settings system. Fixes #1731. --- lib/settings.coffee | 1 + 1 file changed, 1 insertion(+) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index ca4e77b0..842f7618 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -146,6 +146,7 @@ Settings = """ newTabUrl: "chrome://newtab" grabBackFocus: false + regexFindMode: false settingsVersion: Utils.getCurrentVersion() helpDialog_showAdvancedCommands: false -- cgit v1.2.3 From 8ff1aef751a533c17e683207dae1eb165b210f92 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 17 Jun 2015 05:08:03 +0100 Subject: Fix non-default front-end settings. (@mrmr1993: This is yet another approach to the Settings problem.) With the new Settings implemetation, settings which have a non-default value and which are not in synced storage (that is, they have not been changed since synced storage was introduced) are not currently accessible to content scripts. This commit makes such settings accessible via chrome.storage.local. Important: - There's a change to the established settings data model here. Previously, settings with default values were not stored; here, they are. This eliminates a considerable amount logic from Settings, but means that migrations will be required if default values are changed in future. (Other than type changes, have we ever changed a default value?) - There's also a change (bug fix?) to the behaviour when an affected setting is reset to its default value. Previously, the change would *not* have been synced (whereas all other changes are). Here, such changes *are* synced. The previous behaviour was inconsistent with the syncing behaviour of all other options changes. Note: - This isn't particularly well tested. It's being committed mainly just for consideration of the approach, initially. --- lib/settings.coffee | 68 ++++++++++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 35 deletions(-) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index 842f7618..adbe2bef 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -6,19 +6,16 @@ Settings = onLoadedCallbacks: [] init: -> - 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 - @onLoaded() + chrome.storage.local.get null, (localItems) => + localItems = {} if chrome.runtime.lastError + @storage.get null, (syncedItems) => + unless chrome.runtime.lastError + @handleUpdateFromChromeStorage key, value for own key, value of extend localItems, syncedItems - @storage.get null, (items) => - unless chrome.runtime.lastError - @handleUpdateFromChromeStorage key, value for own key, value of items + chrome.storage.onChanged.addListener (changes, area) => + @propagateChangesFromChromeStorage changes if area == "sync" - chrome.storage.onChanged.addListener (changes, area) => - @propagateChangesFromChromeStorage changes if area == "sync" - - @onLoaded() + @onLoaded() # Called after @cache has been initialized. On extension pages, this will be called twice, but that does # not matter because it's idempotent. @@ -37,38 +34,25 @@ Settings = # 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 + value ?= JSON.stringify @defaults[key] + @set key, JSON.parse(value), false 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] - 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 JSON.stringify(value) == JSON.stringify @defaults[key] - @clear key - else - jsonValue = JSON.stringify value - @cache[key] = jsonValue - if @shouldSyncKey key - setting = {}; setting[key] = jsonValue + set: (key, value, shouldSetInSyncedStorage = true) -> + @cache[key] = JSON.stringify value + if @shouldSyncKey key + if shouldSetInSyncedStorage + setting = {}; setting[key] = @cache[key] @storage.set setting - @performPostUpdateHook key, value + # Remove settings installed by the "copyNonDefaultsToChromeStorage-20150717" migration; see below. + chrome.storage.local.remove key if Utils.isBackgroundPage() + @performPostUpdateHook key, value clear: (key) -> - delete @cache[key] if @has key - @storage.remove key if @shouldSyncKey key - @performPostUpdateHook key, @get key + @set key, @defaults[key] has: (key) -> key of @cache @@ -169,5 +153,19 @@ if Utils.isBackgroundPage() rawQuery = Settings.get "findModeRawQuery" chrome.storage.local.set findModeRawQueryList: (if rawQuery then [ rawQuery ] else []) + # Migration (after 1.51, 2015/6/17). + # Copy settings with non-default values (and which are not in synced storage) to chrome.storage.local; + # thereby making these settings accessible within content scripts. + do (migrationKey = "copyNonDefaultsToChromeStorage-20150717") -> + unless localStorage[migrationKey] + chrome.storage.sync.get null, (items) -> + unless chrome.runtime.lastError + updates = {} + for own key of localStorage + if Settings.shouldSyncKey(key) and not items[key] + updates[key] = localStorage[key] + chrome.storage.local.set updates, -> + localStorage[migrationKey] = not chrome.runtime.lastError + root = exports ? window root.Settings = Settings -- cgit v1.2.3 From 84150641a236b30fe5f9c36a0249dbba54dc6166 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 17 Jun 2015 07:57:49 +0100 Subject: Reinstate unintentionally deleted code. --- lib/settings.coffee | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index adbe2bef..f0cf30f1 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -6,6 +6,11 @@ Settings = onLoadedCallbacks: [] init: -> + 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 + @onLoaded() + chrome.storage.local.get null, (localItems) => localItems = {} if chrome.runtime.lastError @storage.get null, (syncedItems) => -- cgit v1.2.3 From 46e19df644b4b7e4040f1d8cee33116d9b900f0d Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 20 Jun 2015 07:16:01 +0100 Subject: Document options/settings data model. --- 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 f0cf30f1..ca6cef89 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -1,4 +1,15 @@ +# A "setting" is a stored key/value pair. An "option" is setting which has a default value and whose value +# can be changed on the options page. +# +# Option values which have never been changed by the user are in Settings.defaults. +# +# Settings whose values have been changed are: +# 1. stored either in chrome.storage.sync or in chrome.storage.local (but never both), and +# 2. cached in Settings.cache; on extension pages, Settings.cache uses localStorage (so it persists). +# +# In all cases except Settings.defaults, values are stored as jsonified strings. + Settings = storage: chrome.storage.sync cache: {} @@ -159,7 +170,7 @@ if Utils.isBackgroundPage() chrome.storage.local.set findModeRawQueryList: (if rawQuery then [ rawQuery ] else []) # Migration (after 1.51, 2015/6/17). - # Copy settings with non-default values (and which are not in synced storage) to chrome.storage.local; + # Copy options with non-default values (and which are not in synced storage) to chrome.storage.local; # thereby making these settings accessible within content scripts. do (migrationKey = "copyNonDefaultsToChromeStorage-20150717") -> unless localStorage[migrationKey] -- cgit v1.2.3 From de770c026a2e023f7be31f2ef1fa5c874578fe29 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 20 Jun 2015 07:27:11 +0100 Subject: Document options/settings data model. --- 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 ca6cef89..5955e8ae 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -1,5 +1,5 @@ -# A "setting" is a stored key/value pair. An "option" is setting which has a default value and whose value +# A "setting" is a stored key/value pair. An "option" is a setting which has a default value and whose value # can be changed on the options page. # # Option values which have never been changed by the user are in Settings.defaults. -- cgit v1.2.3 From aa08e16e6613d5a3760761ab89557df41a50f784 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 20 Jun 2015 07:47:09 +0100 Subject: Adding debugging infrastructure for settings. --- lib/settings.coffee | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/settings.coffee b/lib/settings.coffee index 5955e8ae..437e4d45 100644 --- a/lib/settings.coffee +++ b/lib/settings.coffee @@ -11,6 +11,7 @@ # In all cases except Settings.defaults, values are stored as jsonified strings. Settings = + debug: false storage: chrome.storage.sync cache: {} isLoaded: false @@ -36,6 +37,7 @@ Settings = # Called after @cache has been initialized. On extension pages, this will be called twice, but that does # not matter because it's idempotent. onLoaded: -> + @log "onLoaded: #{@onLoadedCallbacks.length} callback(s)" @isLoaded = true callback() while callback = @onLoadedCallbacks.pop() @@ -46,6 +48,7 @@ Settings = @handleUpdateFromChromeStorage key, change?.newValue for own key, change of changes handleUpdateFromChromeStorage: (key, value) -> + @log "handleUpdateFromChromeStorage: #{key}" # 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 @@ -59,20 +62,26 @@ Settings = set: (key, value, shouldSetInSyncedStorage = true) -> @cache[key] = JSON.stringify value + @log "set: #{key} (length=#{@cache[key].length}, shouldSetInSyncedStorage=#{shouldSetInSyncedStorage})" if @shouldSyncKey key if shouldSetInSyncedStorage setting = {}; setting[key] = @cache[key] + @log " chrome.storage.sync.set(#{key})" @storage.set setting - # Remove settings installed by the "copyNonDefaultsToChromeStorage-20150717" migration; see below. - chrome.storage.local.remove key if Utils.isBackgroundPage() + if Utils.isBackgroundPage() + # Remove options installed by the "copyNonDefaultsToChromeStorage-20150717" migration; see below. + @log " chrome.storage.local.remove(#{key})" + chrome.storage.local.remove key @performPostUpdateHook key, value clear: (key) -> + @log "clear: #{key}" @set key, @defaults[key] has: (key) -> key of @cache use: (key, callback) -> + @log "use: #{key} (isLoaded=#{@isLoaded})" invokeCallback = => callback @get key if @isLoaded then invokeCallback() else @onLoadedCallbacks.push invokeCallback @@ -80,6 +89,10 @@ Settings = postUpdateHooks: {} performPostUpdateHook: (key, value) -> @postUpdateHooks[key]? value + # For development only. + log: (args...) -> + console.log "settings:", args... if @debug + # Default values for all settings. defaults: scrollStepSize: 60 -- cgit v1.2.3