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(+) 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(-) 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 --- content_scripts/vimium_frontend.coffee | 31 ++++++------------------------- lib/settings.coffee | 2 +- manifest.json | 1 + tests/dom_tests/chrome.coffee | 6 +++++- tests/dom_tests/dom_tests.coffee | 30 +++++++++++++++++++----------- tests/dom_tests/dom_tests.html | 1 + 6 files changed, 33 insertions(+), 38 deletions(-) diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index c8c83029..c603e15f 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -65,37 +65,18 @@ settings = searchEngines: null init: -> - @port = chrome.runtime.connect name: "settings" - @port.onMessage.addListener (response) => @receiveMessage response + @port = true + Settings.init() - # If the port is closed, the background page has gone away (since we never close it ourselves). Stub the - # settings object so we don't keep trying to connect to the extension even though it's gone away. - @port.onDisconnect.addListener => - @port = null - for own property, value of this - # @get doesn't depend on @port, so we can continue to support it to try and reduce errors. - @[property] = (->) if "function" == typeof value and property != "get" - - get: (key) -> @values[key] + get: Settings.get.bind Settings set: (key, value) -> @init() unless @port + Settings.set key, value - @values[key] = value - @port.postMessage operation: "set", key: key, value: value - - load: -> - @init() unless @port - @port.postMessage operation: "fetch", values: @values - - receiveMessage: (response) -> - @values = response.values if response.values? - @values[response.key] = response.value if response.key? and response.value? - @isLoaded = true - listener() while listener = @eventListeners.load?.pop() + load: -> @init() unless @port - addEventListener: (eventName, callback) -> - (@eventListeners[eventName] ||= []).push callback + addEventListener: Settings.addEventListener.bind Settings # # Give this frame a unique (non-zero) id. 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 diff --git a/manifest.json b/manifest.json index f0c51117..80eef6c1 100644 --- a/manifest.json +++ b/manifest.json @@ -41,6 +41,7 @@ "lib/rect.js", "lib/handler_stack.js", "lib/clipboard.js", + "lib/settings.js", "content_scripts/ui_component.js", "content_scripts/link_hints.js", "content_scripts/vomnibar.js", diff --git a/tests/dom_tests/chrome.coffee b/tests/dom_tests/chrome.coffee index 4c9bfa52..4de85876 100644 --- a/tests/dom_tests/chrome.coffee +++ b/tests/dom_tests/chrome.coffee @@ -7,6 +7,9 @@ root.chromeMessages = [] document.hasFocus = -> true +fakeManifest = + version: "1.51" + root.chrome = runtime: connect: -> @@ -18,7 +21,7 @@ root.chrome = onMessage: addListener: -> sendMessage: (message) -> chromeMessages.unshift message - getManifest: -> + getManifest: -> fakeManifest getURL: (url) -> "../../#{url}" storage: local: @@ -31,3 +34,4 @@ root.chrome = addListener: -> extension: inIncognitoContext: false + getURL: (url) -> chrome.runtime.getURL url diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee index 8c2b73c3..e57f3eab 100644 --- a/tests/dom_tests/dom_tests.coffee +++ b/tests/dom_tests/dom_tests.coffee @@ -34,12 +34,20 @@ initializeModeState = -> handlerStack.bubbleEvent "registerKeyQueue", keyQueue: "" +# Tell Settings that it's been loaded. +Settings.isLoaded = true + +# Shoulda.js doesn't support async code, so we try not to use any. +Utils.nextTick = (func) -> func() + # # Retrieve the hint markers as an array object. # getHintMarkers = -> Array::slice.call document.getElementsByClassName("vimiumHintMarker"), 0 +stubSettings = (key, value) -> stub Settings.cache, key, JSON.stringify value + # # Generate tests that are common to both default and filtered # link hinting modes. @@ -52,8 +60,8 @@ createGeneralHintTests = (isFilteredMode) -> initializeModeState() testContent = "test" + "tress" document.getElementById("test-div").innerHTML = testContent - stub settings.values, "filterLinkHints", false - stub settings.values, "linkHintCharacters", "ab" + stubSettings "filterLinkHints", false + stubSettings "linkHintCharacters", "ab" tearDown -> document.getElementById("test-div").innerHTML = "" @@ -92,8 +100,8 @@ context "Test link hints for focusing input elements correctly", testDiv = document.getElementById("test-div") testDiv.innerHTML = "" - stub settings.values, "filterLinkHints", false - stub settings.values, "linkHintCharacters", "ab" + stubSettings "filterLinkHints", false + stubSettings "linkHintCharacters", "ab" # Every HTML5 input type except for hidden. We should be able to activate all of them with link hints. inputTypes = ["button", "checkbox", "color", "date", "datetime", "datetime-local", "email", "file", @@ -129,8 +137,8 @@ context "Alphabetical link hints", setup -> initializeModeState() - stub settings.values, "filterLinkHints", false - stub settings.values, "linkHintCharacters", "ab" + stubSettings "filterLinkHints", false + stubSettings "linkHintCharacters", "ab" # Three hints will trigger double hint chars. createLinks 3 @@ -161,8 +169,8 @@ context "Filtered link hints", # elements. setup -> - stub settings.values, "filterLinkHints", true - stub settings.values, "linkHintNumbers", "0123456789" + stubSettings "filterLinkHints", true + stubSettings "linkHintNumbers", "0123456789" context "Text hints", @@ -289,7 +297,7 @@ context "Find prev / next links", nextcorrupted next page """ - stub settings.values, "nextPatterns", "next" + stubSettings "nextPatterns", "next" goNext() assert.equal '#second', window.location.hash @@ -297,7 +305,7 @@ context "Find prev / next links", document.getElementById("test-div").innerHTML = """ >> """ - stub settings.values, "nextPatterns", ">>" + stubSettings "nextPatterns", ">>" goNext() assert.equal '#first', window.location.hash @@ -306,7 +314,7 @@ context "Find prev / next links", lorem ipsum next next! """ - stub settings.values, "nextPatterns", "next" + stubSettings "nextPatterns", "next" goNext() assert.equal '#second', window.location.hash diff --git a/tests/dom_tests/dom_tests.html b/tests/dom_tests/dom_tests.html index 5ccd39e7..f7cc430d 100644 --- a/tests/dom_tests/dom_tests.html +++ b/tests/dom_tests/dom_tests.html @@ -35,6 +35,7 @@ + -- cgit v1.2.3 From 4b420fe89502ce910d4cc13fda51e0a8ad06fed9 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 17:55:32 +0100 Subject: Replace settings.get with Settings.get in the frontend --- content_scripts/hud.coffee | 2 +- content_scripts/link_hints.coffee | 10 +++++----- content_scripts/scroller.coffee | 4 ++-- content_scripts/vimium_frontend.coffee | 20 +++++++++----------- content_scripts/vomnibar.coffee | 2 +- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/content_scripts/hud.coffee b/content_scripts/hud.coffee index f38d6b45..84b8abeb 100644 --- a/content_scripts/hud.coffee +++ b/content_scripts/hud.coffee @@ -48,7 +48,7 @@ HUD = -> ready and document.body != null # A preference which can be toggled in the Options page. */ - enabled: -> !settings.get("hideHud") + enabled: -> !Settings.get("hideHud") class Tween opacity: 0 diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 3cebac4c..daf738a3 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -29,7 +29,7 @@ LinkHints = # Handle the link hinting marker generation and matching. Must be initialized after settings have been # loaded, so that we can retrieve the option setting. getMarkerMatcher: -> - if settings.get("filterLinkHints") then filterHints else alphabetHints + if Settings.get("filterLinkHints") then filterHints else alphabetHints # lock to ensure only one instance runs at a time isActive: false # Call this function on exit (if defined). @@ -60,7 +60,7 @@ LinkHints = # For these modes, we filter out those elements which don't have an HREF (since there's nothing we can do # with them). elements = (el for el in elements when el.element.href?) if mode in [ COPY_LINK_URL, OPEN_INCOGNITO ] - if settings.get "filterLinkHints" + if Settings.get "filterLinkHints" # When using text filtering, we sort the elements such that we visit descendants before their ancestors. # This allows us to exclude the text used for matching descendants from that used for matching their # ancestors. @@ -389,7 +389,7 @@ alphabetHints = # may be of different lengths. # hintStrings: (linkCount) -> - linkHintCharacters = settings.get("linkHintCharacters") + linkHintCharacters = Settings.get("linkHintCharacters") # Determine how many digits the link hints will require in the worst case. Usually we do not need # all of these digits for every link single hint, so we can show shorter hints for a few of the links. digitsNeeded = Math.ceil(@logXOfBase(linkCount, linkHintCharacters.length)) @@ -460,7 +460,7 @@ filterHints = @labelMap[forElement] = labelText generateHintString: (linkHintNumber) -> - (numberToHintString linkHintNumber + 1, settings.get "linkHintNumbers").toUpperCase() + (numberToHintString linkHintNumber + 1, Settings.get "linkHintNumbers").toUpperCase() generateLinkText: (element) -> linkText = "" @@ -519,7 +519,7 @@ filterHints = if (!@hintKeystrokeQueue.pop() && !@linkTextKeystrokeQueue.pop()) return { linksMatched: [] } else if (keyChar) - if (settings.get("linkHintNumbers").indexOf(keyChar) >= 0) + if (Settings.get("linkHintNumbers").indexOf(keyChar) >= 0) @hintKeystrokeQueue.push(keyChar) else # since we might renumber the hints, the current hintKeyStrokeQueue diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index 27fc9cdc..29142064 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -139,7 +139,7 @@ CoreScroller = @time += 1 # Return true if CoreScroller would not initiate a new scroll right now. - wouldNotInitiateScroll: -> @lastEvent?.repeat and @settings.get "smoothScroll" + wouldNotInitiateScroll: -> @lastEvent?.repeat and Settings.get "smoothScroll" # Calibration fudge factors for continuous scrolling. The calibration value starts at 1.0. We then # increase it (until it exceeds @maxCalibration) if we guess that the scroll is too slow, or decrease it @@ -153,7 +153,7 @@ CoreScroller = scroll: (element, direction, amount, continuous = true) -> return unless amount - unless @settings.get "smoothScroll" + unless Settings.get "smoothScroll" # Jump scrolling. performScroll element, direction, amount checkVisibility element diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index c603e15f..23b725e4 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -68,8 +68,6 @@ settings = @port = true Settings.init() - get: Settings.get.bind Settings - set: (key, value) -> @init() unless @port Settings.set key, value @@ -101,7 +99,7 @@ class GrabBackFocus extends Mode mousedown: => @alwaysContinueBubbling => @exit() activate = => - return @exit() unless settings.get "grabBackFocus" + return @exit() unless Settings.get "grabBackFocus" @push _name: "grab-back-focus-focus" focus: (event) => @grabBackFocus event.target @@ -345,14 +343,14 @@ extend window, scrollToTop: -> Scroller.scrollTo "y", 0 scrollToLeft: -> Scroller.scrollTo "x", 0 scrollToRight: -> Scroller.scrollTo "x", "max" - scrollUp: -> Scroller.scrollBy "y", -1 * settings.get("scrollStepSize") - scrollDown: -> Scroller.scrollBy "y", settings.get("scrollStepSize") + scrollUp: -> Scroller.scrollBy "y", -1 * Settings.get("scrollStepSize") + scrollDown: -> Scroller.scrollBy "y", Settings.get("scrollStepSize") scrollPageUp: -> Scroller.scrollBy "y", "viewSize", -1/2 scrollPageDown: -> Scroller.scrollBy "y", "viewSize", 1/2 scrollFullPageUp: -> Scroller.scrollBy "y", "viewSize", -1 scrollFullPageDown: -> Scroller.scrollBy "y", "viewSize" - scrollLeft: -> Scroller.scrollBy "x", -1 * settings.get("scrollStepSize") - scrollRight: -> Scroller.scrollBy "x", settings.get("scrollStepSize") + scrollLeft: -> Scroller.scrollBy "x", -1 * Settings.get("scrollStepSize") + scrollRight: -> Scroller.scrollBy "x", Settings.get("scrollStepSize") extend window, reload: -> window.location.reload() @@ -698,7 +696,7 @@ updateFindModeQuery = -> # the query can be treated differently (e.g. as a plain string versus regex depending on the presence of # escape sequences. '\' is the escape character and needs to be escaped itself to be used as a normal # character. here we grep for the relevant escape sequences. - findModeQuery.isRegex = settings.get 'regexFindMode' + findModeQuery.isRegex = Settings.get 'regexFindMode' hasNoIgnoreCaseFlag = false findModeQuery.parsedQuery = findModeQuery.rawQuery.replace /(\\{1,2})([rRI]?)/g, (match, slashes, flag) -> return match if flag == "" or slashes.length != 1 @@ -1010,12 +1008,12 @@ findAndFollowRel = (value) -> return true window.goPrevious = -> - previousPatterns = settings.get("previousPatterns") || "" + previousPatterns = Settings.get("previousPatterns") || "" previousStrings = previousPatterns.split(",").filter( (s) -> s.trim().length ) findAndFollowRel("prev") || findAndFollowLink(previousStrings) window.goNext = -> - nextPatterns = settings.get("nextPatterns") || "" + nextPatterns = Settings.get("nextPatterns") || "" nextStrings = nextPatterns.split(",").filter( (s) -> s.trim().length ) findAndFollowRel("next") || findAndFollowLink(nextStrings) @@ -1070,7 +1068,7 @@ window.showHelpDialog = (html, fid) -> VimiumHelpDialog = # This setting is pulled out of local storage. It's false by default. - getShowAdvancedCommands: -> settings.get("helpDialog_showAdvancedCommands") + getShowAdvancedCommands: -> Settings.get("helpDialog_showAdvancedCommands") init: () -> this.dialogElement = document.getElementById("vimiumHelpDialog") diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee index 4bd8e8fd..6c08ce92 100644 --- a/content_scripts/vomnibar.coffee +++ b/content_scripts/vomnibar.coffee @@ -8,7 +8,7 @@ Vomnibar = # the form "keyword=X", for direct activation of a custom search engine. parseRegistryEntry: (registryEntry = { options: [] }, callback = null) -> options = {} - searchEngines = settings.get("searchEngines") ? "" + searchEngines = Settings.get("searchEngines") ? "" SearchEngines.refreshAndUse searchEngines, (engines) -> for option in registryEntry.options [ key, value ] = option.split "=" -- cgit v1.2.3 From 2929935c1ebb093797c2b5d02153ec18e63a2c24 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 17:56:32 +0100 Subject: Replace settings.set with Settings.set in the frontend --- content_scripts/vimium_frontend.coffee | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 23b725e4..90c2b09d 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -68,10 +68,6 @@ settings = @port = true Settings.init() - set: (key, value) -> - @init() unless @port - Settings.set key, value - load: -> @init() unless @port addEventListener: Settings.addEventListener.bind Settings @@ -1084,7 +1080,7 @@ window.showHelpDialog = (html, fid) -> event.preventDefault() showAdvanced = VimiumHelpDialog.getShowAdvancedCommands() VimiumHelpDialog.showAdvancedCommands(!showAdvanced) - settings.set("helpDialog_showAdvancedCommands", !showAdvanced) + Settings.set("helpDialog_showAdvancedCommands", !showAdvanced) showAdvancedCommands: (visible) -> VimiumHelpDialog.dialogElement.getElementsByClassName("toggleAdvancedCommands")[0].innerHTML = -- cgit v1.2.3 From c17914f59a27779d102a80ccee27f08298b7c015 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 17:59:27 +0100 Subject: Replace settings.addEventListener with Settings.addEventListener in the frontend --- content_scripts/vimium_frontend.coffee | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 90c2b09d..17922659 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -70,8 +70,6 @@ settings = load: -> @init() unless @port - addEventListener: Settings.addEventListener.bind Settings - # # Give this frame a unique (non-zero) id. # @@ -102,7 +100,7 @@ class GrabBackFocus extends Mode # An input may already be focused. If so, grab back the focus. @grabBackFocus document.activeElement if document.activeElement - if settings.isLoaded then activate() else settings.addEventListener "load", activate + if settings.isLoaded then activate() else Settings.addEventListener "load", activate grabBackFocus: (element) -> return @continueBubbling unless DomUtils.isEditable element @@ -157,7 +155,7 @@ window.initializeModes = -> # Complete initialization work that sould be done prior to DOMReady. # initializePreDomReady = -> - settings.addEventListener("load", LinkHints.init.bind(LinkHints)) + Settings.addEventListener "load", LinkHints.init.bind LinkHints settings.load() initializeModes() -- cgit v1.2.3 From 7e67c3d36b47641b21ec24be5d8f7af11ad08756 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 18:01:42 +0100 Subject: Init Settings directly instead of via settings.load, and only do it once --- content_scripts/vimium_frontend.coffee | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 17922659..5655ef61 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -68,8 +68,6 @@ settings = @port = true Settings.init() - load: -> @init() unless @port - # # Give this frame a unique (non-zero) id. # @@ -156,7 +154,7 @@ window.initializeModes = -> # initializePreDomReady = -> Settings.addEventListener "load", LinkHints.init.bind LinkHints - settings.load() + Settings.init() initializeModes() checkIfEnabledForUrl() @@ -240,7 +238,6 @@ window.installListeners = -> # onFocus = (event) -> if event.target == window - settings.load() chrome.runtime.sendMessage handler: "frameFocused", frameId: frameId checkIfEnabledForUrl true -- cgit v1.2.3 From f09f65c53cddf05544c8fa417bb0d92438c98a63 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 18:10:01 +0100 Subject: Remove all remaining references to frontend settings --- content_scripts/scroller.coffee | 7 +++---- content_scripts/vimium_frontend.coffee | 11 ++--------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee index 29142064..81c71fcd 100644 --- a/content_scripts/scroller.coffee +++ b/content_scripts/scroller.coffee @@ -117,8 +117,7 @@ checkVisibility = (element) -> # CoreScroller contains the core function (scroll) and logic for relative scrolls. All scrolls are ultimately # translated to relative scrolls. CoreScroller is not exported. CoreScroller = - init: (frontendSettings) -> - @settings = frontendSettings + init: -> @time = 0 @lastEvent = null @keyIsDown = false @@ -215,11 +214,11 @@ CoreScroller = # Scroller contains the two main scroll functions which are used by clients. Scroller = - init: (frontendSettings) -> + init: -> handlerStack.push _name: 'scroller/active-element' DOMActivate: (event) -> handlerStack.alwaysContinueBubbling -> activatedElement = event.target - CoreScroller.init frontendSettings + CoreScroller.init() # scroll the active element in :direction by :amount * :factor. # :factor is needed because :amount can take on string values, which scrollBy converts to element dimensions. diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 5655ef61..b6c61c04 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -46,9 +46,6 @@ textInputXPath = (-> # must be called beforehand to ensure get() will return up-to-date values. # settings = - isLoaded: false - port: null - eventListeners: {} values: scrollStepSize: null linkHintCharacters: null @@ -64,10 +61,6 @@ settings = grabBackFocus: null searchEngines: null - init: -> - @port = true - Settings.init() - # # Give this frame a unique (non-zero) id. # @@ -98,7 +91,7 @@ class GrabBackFocus extends Mode # An input may already be focused. If so, grab back the focus. @grabBackFocus document.activeElement if document.activeElement - if settings.isLoaded then activate() else Settings.addEventListener "load", activate + if Settings.isLoaded then activate() else Settings.addEventListener "load", activate grabBackFocus: (element) -> return @continueBubbling unless DomUtils.isEditable element @@ -147,7 +140,7 @@ window.initializeModes = -> new NormalMode new PassKeysMode new InsertMode permanent: true - Scroller.init settings + Scroller.init() # # Complete initialization work that sould be done prior to DOMReady. -- cgit v1.2.3 From c62bee9036c05beb7945a0a9088e848617960f26 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 18:14:29 +0100 Subject: Update the comment by the settings object in the frontend --- content_scripts/vimium_frontend.coffee | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index b6c61c04..47ee9e68 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -41,10 +41,9 @@ textInputXPath = (-> DomUtils.makeXPath(inputElements) )() -# -# settings provides a browser-global localStorage-backed dict. get() and set() are synchronous, but load() -# must be called beforehand to ensure get() will return up-to-date values. -# +# NOTE(mrmr1993): we use Settings everywhere instead of the dedicated implementation that was here. +# Previously, only the values listed below would be loaded. If the space used by settings across all of our +# content scripts is becoming an issue, then we can restrict the values we load to the list below. settings = values: scrollStepSize: null -- cgit v1.2.3 From dbf0ed0956485ccd3c0fb3fb12cf12c007d63394 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sun, 31 May 2015 18:16:01 +0100 Subject: Remove code supporting the former settings port from the frontend --- background_scripts/main.coffee | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index 99a5672b..e40b03be 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -217,20 +217,6 @@ selectSpecificTab = (request) -> chrome.windows.update(tab.windowId, { focused: true }) chrome.tabs.update(request.id, { selected: true })) -# -# Used by the content scripts to get settings from the local storage. -# -handleSettings = (request, port) -> - switch request.operation - when "get" # Get a single settings value. - port.postMessage key: request.key, value: Settings.get request.key - when "set" # Set a single settings value. - Settings.set request.key, request.value - when "fetch" # Fetch multiple settings values. - values = request.values - values[key] = Settings.get key for own key of values - port.postMessage { values } - chrome.tabs.onSelectionChanged.addListener (tabId, selectionInfo) -> if (selectionChangedHandlers.length > 0) selectionChangedHandlers.pop().call() @@ -650,7 +636,6 @@ bgLog = (request, sender) -> # Port handler mapping portHandlers = keyDown: handleKeyDown, - settings: handleSettings, completions: handleCompletions sendRequestHandlers = -- 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(+) 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. --- background_scripts/main.coffee | 2 - content_scripts/link_hints.coffee | 2 +- content_scripts/vimium_frontend.coffee | 42 +++++-------------- lib/settings.coffee | 74 +++++++++++++++------------------- pages/options.coffee | 1 - tests/dom_tests/chrome.coffee | 2 +- tests/unit_tests/exclusion_test.coffee | 1 - tests/unit_tests/settings_test.coffee | 1 - tests/unit_tests/utils_test.coffee | 1 - 9 files changed, 44 insertions(+), 82 deletions(-) diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee index e40b03be..980f8e18 100644 --- a/background_scripts/main.coffee +++ b/background_scripts/main.coffee @@ -728,6 +728,4 @@ chrome.windows.getAll { populate: true }, (windows) -> (response) -> updateScrollPosition(tab, response.scrollX, response.scrollY) if response? chrome.tabs.sendMessage(tab.id, { name: "getScrollPosition" }, createScrollPositionHandler()) -# Start pulling changes from synchronized storage. -Settings.init() showUpgradeMessage() diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index daf738a3..0ea40bd3 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -26,7 +26,7 @@ LinkHints = linkActivator: undefined # While in delayMode, all keypresses have no effect. delayMode: false - # Handle the link hinting marker generation and matching. Must be initialized after settings have been + # Handle the link hinting marker generation and matching. Must be initialized after Settings have been # loaded, so that we can retrieve the option setting. getMarkerMatcher: -> if Settings.get("filterLinkHints") then filterHints else alphabetHints diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 47ee9e68..bb1e971f 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -41,25 +41,6 @@ textInputXPath = (-> DomUtils.makeXPath(inputElements) )() -# NOTE(mrmr1993): we use Settings everywhere instead of the dedicated implementation that was here. -# Previously, only the values listed below would be loaded. If the space used by settings across all of our -# content scripts is becoming an issue, then we can restrict the values we load to the list below. -settings = - values: - scrollStepSize: null - linkHintCharacters: null - linkHintNumbers: null - filterLinkHints: null - hideHud: null - previousPatterns: null - nextPatterns: null - regexFindMode: null - userDefinedLinkHintCss: null - helpDialog_showAdvancedCommands: null - smoothScroll: null - grabBackFocus: null - searchEngines: null - # # Give this frame a unique (non-zero) id. # @@ -82,15 +63,15 @@ class GrabBackFocus extends Mode _name: "grab-back-focus-mousedown" mousedown: => @alwaysContinueBubbling => @exit() - activate = => - return @exit() unless Settings.get "grabBackFocus" - @push - _name: "grab-back-focus-focus" - focus: (event) => @grabBackFocus event.target - # An input may already be focused. If so, grab back the focus. - @grabBackFocus document.activeElement if document.activeElement - - if Settings.isLoaded then activate() else Settings.addEventListener "load", activate + Settings.use "grabBackFocus", (grabBackFocus) => + if grabBackFocus + @push + _name: "grab-back-focus-focus" + focus: (event) => @grabBackFocus event.target + # An input may already be focused. If so, grab back the focus. + @grabBackFocus document.activeElement if document.activeElement + else + @exit() grabBackFocus: (element) -> return @continueBubbling unless DomUtils.isEditable element @@ -145,8 +126,7 @@ window.initializeModes = -> # Complete initialization work that sould be done prior to DOMReady. # initializePreDomReady = -> - Settings.addEventListener "load", LinkHints.init.bind LinkHints - Settings.init() + Settings.use "theKeyHereDoesNotMatter", LinkHints.init.bind LinkHints initializeModes() checkIfEnabledForUrl() @@ -224,7 +204,6 @@ window.installListeners = -> # # Whenever we get the focus: -# - Reload settings (they may have changed). # - Tell the background page this frame's URL. # - Check if we should be enabled. # @@ -1146,7 +1125,6 @@ window.onbeforeunload = -> scrollY: window.scrollY) root = exports ? window -root.settings = settings root.handlerStack = handlerStack root.frameId = frameId root.windowIsFocused = windowIsFocused 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 diff --git a/pages/options.coffee b/pages/options.coffee index c8c21850..ddab2bf4 100644 --- a/pages/options.coffee +++ b/pages/options.coffee @@ -1,6 +1,5 @@ $ = (id) -> document.getElementById id -Settings.init() bgExclusions = chrome.extension.getBackgroundPage().Exclusions # diff --git a/tests/dom_tests/chrome.coffee b/tests/dom_tests/chrome.coffee index 4de85876..d4e6930d 100644 --- a/tests/dom_tests/chrome.coffee +++ b/tests/dom_tests/chrome.coffee @@ -28,7 +28,7 @@ root.chrome = get: -> set: -> sync: - get: -> + get: (_, callback) -> callback? {} set: -> onChanged: addListener: -> diff --git a/tests/unit_tests/exclusion_test.coffee b/tests/unit_tests/exclusion_test.coffee index 28c17a2f..0e4b87bc 100644 --- a/tests/unit_tests/exclusion_test.coffee +++ b/tests/unit_tests/exclusion_test.coffee @@ -15,7 +15,6 @@ root.Marks = extend(global, require "../../lib/utils.js") Utils.getCurrentVersion = -> '1.44' extend(global,require "../../lib/settings.js") -Settings.init() extend(global, require "../../background_scripts/exclusions.js") extend(global, require "../../background_scripts/commands.js") extend(global, require "../../background_scripts/main.js") diff --git a/tests/unit_tests/settings_test.coffee b/tests/unit_tests/settings_test.coffee index ded7b5f8..a2aca6fd 100644 --- a/tests/unit_tests/settings_test.coffee +++ b/tests/unit_tests/settings_test.coffee @@ -14,7 +14,6 @@ context "settings", stub global, 'localStorage', {} Settings.cache = global.localStorage # Point the settings cache to the new localStorage object. Settings.postUpdateHooks = {} # Avoid running update hooks which include calls to outside of settings. - Settings.init() should "save settings in localStorage as JSONified strings", -> Settings.set 'dummy', "" diff --git a/tests/unit_tests/utils_test.coffee b/tests/unit_tests/utils_test.coffee index f9ed3636..67c3b333 100644 --- a/tests/unit_tests/utils_test.coffee +++ b/tests/unit_tests/utils_test.coffee @@ -3,7 +3,6 @@ extend global, require "./test_chrome_stubs.js" extend(global, require "../../lib/utils.js") Utils.getCurrentVersion = -> '1.43' extend(global, require "../../lib/settings.js") -Settings.init() context "isUrl", should "accept valid URLs", -> -- cgit v1.2.3 From e8476682362b9648aba874e8581fe9076479f734 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 1 Jun 2015 09:46:41 +0100 Subject: Remove LinkHints.init()... LinkHints.init() isn't doing anything. --- content_scripts/link_hints.coffee | 5 ----- content_scripts/vimium_frontend.coffee | 2 -- tests/dom_tests/dom_tests.coffee | 2 -- 3 files changed, 9 deletions(-) diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 0ea40bd3..2bcc7508 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -35,11 +35,6 @@ LinkHints = # Call this function on exit (if defined). onExit: null - # - # To be called after linkHints has been generated from linkHintsBase. - # - init: -> - # We need this as a top-level function because our command system doesn't yet support arguments. activateModeToOpenInNewTab: -> @activateMode(OPEN_IN_NEW_BG_TAB) activateModeToOpenInNewForegroundTab: -> @activateMode(OPEN_IN_NEW_FG_TAB) diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index bb1e971f..8000a9ec 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -126,8 +126,6 @@ window.initializeModes = -> # Complete initialization work that sould be done prior to DOMReady. # initializePreDomReady = -> - Settings.use "theKeyHereDoesNotMatter", LinkHints.init.bind LinkHints - initializeModes() checkIfEnabledForUrl() refreshCompletionKeys() diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee index e57f3eab..8f293075 100644 --- a/tests/dom_tests/dom_tests.coffee +++ b/tests/dom_tests/dom_tests.coffee @@ -142,7 +142,6 @@ context "Alphabetical link hints", # Three hints will trigger double hint chars. createLinks 3 - LinkHints.init() LinkHints.activateMode() tearDown -> @@ -178,7 +177,6 @@ context "Filtered link hints", initializeModeState() testContent = "test" + "tress" + "trait" + "trackalt text" document.getElementById("test-div").innerHTML = testContent - LinkHints.init() LinkHints.activateMode() tearDown -> -- 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(-) 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 +++++++++++------------------- tests/unit_tests/settings_test.coffee | 11 ++- tests/unit_tests/test_chrome_stubs.coffee | 4 +- 3 files changed, 65 insertions(+), 99 deletions(-) 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 diff --git a/tests/unit_tests/settings_test.coffee b/tests/unit_tests/settings_test.coffee index a2aca6fd..08145190 100644 --- a/tests/unit_tests/settings_test.coffee +++ b/tests/unit_tests/settings_test.coffee @@ -38,16 +38,23 @@ context "settings", Settings.clear 'scrollStepSize' assert.equal Settings.get('scrollStepSize'), 60 +context "synced settings", + + setup -> + stub global, 'localStorage', {} + Settings.cache = global.localStorage # Point the settings cache to the new localStorage object. + Settings.postUpdateHooks = {} # Avoid running update hooks which include calls to outside of settings. + should "propagate non-default value via synced storage listener", -> Settings.set 'scrollStepSize', 20 assert.equal Settings.get('scrollStepSize'), 20 - Settings.Sync.handleStorageUpdate { scrollStepSize: { newValue: "40" } } + Settings.propagateChangesFromChromeStorage { scrollStepSize: { newValue: "40" } } assert.equal Settings.get('scrollStepSize'), 40 should "propagate default value via synced storage listener", -> Settings.set 'scrollStepSize', 20 assert.equal Settings.get('scrollStepSize'), 20 - Settings.Sync.handleStorageUpdate { scrollStepSize: { newValue: "60" } } + Settings.propagateChangesFromChromeStorage { scrollStepSize: { newValue: "60" } } assert.isFalse Settings.has 'scrollStepSize' should "propagate non-default values from synced storage", -> diff --git a/tests/unit_tests/test_chrome_stubs.coffee b/tests/unit_tests/test_chrome_stubs.coffee index 16f0e144..fe2fc298 100644 --- a/tests/unit_tests/test_chrome_stubs.coffee +++ b/tests/unit_tests/test_chrome_stubs.coffee @@ -70,14 +70,14 @@ exports.chrome = chrome.runtime.lastError = undefined key_value = {} key_value[key] = { newValue: value } - @func(key_value,'synced storage stub') if @func + @func(key_value,'sync') if @func callEmpty: (key) -> chrome.runtime.lastError = undefined if @func items = {} items[key] = {} - @func(items,'synced storage stub') + @func(items,'sync') session: MAX_SESSION_RESULTS: 25 -- cgit v1.2.3 From 0f90453d269c5f8ab700123aa356a3d41026d925 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 1 Jun 2015 11:35:51 +0100 Subject: Eliminate possibility of race condition. See newly-added long comment for details. --- pages/options.coffee | 13 ++++++++++--- pages/options.html | 1 - 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/pages/options.coffee b/pages/options.coffee index ddab2bf4..99492291 100644 --- a/pages/options.coffee +++ b/pages/options.coffee @@ -2,6 +2,12 @@ $ = (id) -> document.getElementById id bgExclusions = chrome.extension.getBackgroundPage().Exclusions +# We have to use Settings from the background page here (not Settings, directly) to avoid a race condition for +# the page popup. Specifically, we must ensure that the settings have been updated on the background page +# *before* the popup closes. This ensures that any exclusion-rule changes are in place before the page +# regains the focus. +bgSettings = chrome.extension.getBackgroundPage().Settings + # # Class hierarchy for various types of option. class Option @@ -20,20 +26,21 @@ class Option # Fetch a setting from localStorage, remember the @previous value and populate the DOM element. # Return the fetched value. fetch: -> - @populateElement @previous = Settings.get @field + @populateElement @previous = bgSettings.get @field @previous # Write this option's new value back to localStorage, if necessary. save: -> value = @readValueFromElement() if not @areEqual value, @previous - Settings.set @field, @previous = value + bgSettings.set @field, @previous = value + bgSettings.performPostUpdateHook @field, value # Compare values; this is overridden by sub-classes. areEqual: (a,b) -> a == b restoreToDefault: -> - Settings.clear @field + bgSettings.clear @field @fetch() # Static method. diff --git a/pages/options.html b/pages/options.html index b14c454f..441bd9da 100644 --- a/pages/options.html +++ b/pages/options.html @@ -3,7 +3,6 @@ Vimium Options - -- 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(-) 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(+) 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(-) 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 ++ pages/options.coffee | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) 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 diff --git a/pages/options.coffee b/pages/options.coffee index 99492291..0d7106fa 100644 --- a/pages/options.coffee +++ b/pages/options.coffee @@ -34,7 +34,6 @@ class Option value = @readValueFromElement() if not @areEqual value, @previous bgSettings.set @field, @previous = value - bgSettings.performPostUpdateHook @field, value # Compare values; this is overridden by sub-classes. areEqual: (a,b) -> a == b -- cgit v1.2.3