From 61764d812a37ca2c29b3b7ddde878f25250abf81 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Wed, 10 Jun 2015 16:39:02 +0100 Subject: Fix bug relating to duplicate hint strings. (Not sure when this crept in.) We need to ensure that we always generate the same hint strings for the same filter state. Here, we do this by always using the same mechanism (@filterLinkHints) to set the hint strings. --- tests/dom_tests/dom_tests.coffee | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee index dd2f5a5d..a79735ae 100644 --- a/tests/dom_tests/dom_tests.coffee +++ b/tests/dom_tests/dom_tests.coffee @@ -212,11 +212,14 @@ context "Filtered link hints", @linkHints.deactivateMode() should "label the images", -> - hintMarkers = getHintMarkers() - assert.equal "1: alt text", hintMarkers[0].textContent.toLowerCase() - assert.equal "2: some title", hintMarkers[1].textContent.toLowerCase() - assert.equal "3: alt text", hintMarkers[2].textContent.toLowerCase() - assert.equal "4", hintMarkers[3].textContent.toLowerCase() + hintMarkers = getHintMarkers().map (marker) -> marker.textContent.toLowerCase() + # We don't know the actual hint numbers which will be assigned, so we replace them with "N". + hintMarkers = hintMarkers.map (str) -> str.replace /^[1-4]/, "N" + assert.equal 4, hintMarkers.length + assert.isTrue "N: alt text" in hintMarkers + assert.isTrue "N: some title" in hintMarkers + assert.isTrue "N: alt text" in hintMarkers + assert.isTrue "N" in hintMarkers context "Input hints", @@ -235,11 +238,15 @@ context "Filtered link hints", should "label the input elements", -> hintMarkers = getHintMarkers() - assert.equal "1", hintMarkers[0].textContent.toLowerCase() - assert.equal "2", hintMarkers[1].textContent.toLowerCase() - assert.equal "3: a label", hintMarkers[2].textContent.toLowerCase() - assert.equal "4: a label", hintMarkers[3].textContent.toLowerCase() - assert.equal "5", hintMarkers[4].textContent.toLowerCase() + hintMarkers = getHintMarkers().map (marker) -> marker.textContent.toLowerCase() + # We don't know the actual hint numbers which will be assigned, so we replace them with "N". + hintMarkers = hintMarkers.map (str) -> str.replace /^[1-5]/, "N" + assert.equal 5, hintMarkers.length + assert.isTrue "N" in hintMarkers + assert.isTrue "N" in hintMarkers + assert.isTrue "N: a label" in hintMarkers + assert.isTrue "N: a label" in hintMarkers + assert.isTrue "N" in hintMarkers context "Input focus", -- 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. --- tests/unit_tests/settings_test.coffee | 12 +++--------- tests/unit_tests/test_chrome_stubs.coffee | 6 +++--- 2 files changed, 6 insertions(+), 12 deletions(-) (limited to 'tests') diff --git a/tests/unit_tests/settings_test.coffee b/tests/unit_tests/settings_test.coffee index 08145190..6270ae3e 100644 --- a/tests/unit_tests/settings_test.coffee +++ b/tests/unit_tests/settings_test.coffee @@ -27,12 +27,6 @@ context "settings", Settings.set 'scrollStepSize', 20 assert.equal Settings.get('scrollStepSize'), 20 - should "not store values equal to the default", -> - Settings.set 'scrollStepSize', 20 - assert.isTrue Settings.has 'scrollStepSize' - Settings.set 'scrollStepSize', 60 - assert.isFalse Settings.has 'scrollStepSize' - should "revert to defaults if no key is stored", -> Settings.set 'scrollStepSize', 20 Settings.clear 'scrollStepSize' @@ -55,7 +49,7 @@ context "synced settings", Settings.set 'scrollStepSize', 20 assert.equal Settings.get('scrollStepSize'), 20 Settings.propagateChangesFromChromeStorage { scrollStepSize: { newValue: "60" } } - assert.isFalse Settings.has 'scrollStepSize' + assert.equal Settings.get('scrollStepSize'), 60 should "propagate non-default values from synced storage", -> chrome.storage.sync.set { scrollStepSize: JSON.stringify(20) } @@ -64,12 +58,12 @@ context "synced settings", should "propagate default values from synced storage", -> Settings.set 'scrollStepSize', 20 chrome.storage.sync.set { scrollStepSize: JSON.stringify(60) } - assert.isFalse Settings.has 'scrollStepSize' + assert.equal Settings.get('scrollStepSize'), 60 should "clear a setting from synced storage", -> Settings.set 'scrollStepSize', 20 chrome.storage.sync.remove 'scrollStepSize' - assert.isFalse Settings.has 'scrollStepSize' + assert.equal Settings.get('scrollStepSize'), 60 should "trigger a postUpdateHook", -> message = "Hello World" diff --git a/tests/unit_tests/test_chrome_stubs.coffee b/tests/unit_tests/test_chrome_stubs.coffee index fe2fc298..c6a56521 100644 --- a/tests/unit_tests/test_chrome_stubs.coffee +++ b/tests/unit_tests/test_chrome_stubs.coffee @@ -57,9 +57,9 @@ exports.chrome = storage: # chrome.storage.local local: - get: -> - set: -> - remove: -> + get: (_, callback) -> callback?() + set: (_, callback) -> callback?() + remove: (_, callback) -> callback?() # chrome.storage.onChanged onChanged: -- cgit v1.2.3