search

duckduckgo / firefox-zeroclickinfo

Firefox Add-on using the DuckDuckGo Zero-click Info API
Other
85 stars 27 forks source link

get set_atb param #68

Closed jdorweiler closed 7 years ago

jdorweiler commented 7 years ago

@zachthompson

zachthompson commented 7 years ago

Can you tell me how you were able to test it?

jdorweiler commented 7 years ago

To test I did the following:

  1. set a break point in ddg-atb.js at https://github.com/duckduckgo/firefox-zeroclickinfo/pull/68/files#diff-37db317c80b52c5e2e654df44f106a3aR40
  2. Do a search and stop at break point
  3. manually set the atb localstorage values
  4. Do another search and check to see that the set_atb value has been updated to the current version

I also verified the server side of the request as well.

bsstoner commented 7 years ago

@jdorweiler I think if you want to rename to set_atb, we'd need to add a shim for people that have the old value in localStorage as atb_set, otherwise we won't get any data from people that already have the extension installed?

jdorweiler commented 7 years ago

Good point. I'll stick with atb_set then.

bsstoner commented 7 years ago

In the Chrome extension we moved to setting the ATB value instead of true on install here: https://github.com/duckduckgo/chrome-zeroclickinfo/blob/master/js/background.js#L87

We might want to do the same thing in FF here for consistency: https://github.com/duckduckgo/firefox-zeroclickinfo/pull/68/files#diff-27710c92de8dde607d5dbd5bf1c7d92fL33

zachthompson commented 7 years ago

@bsstoner @mrshu jd says this is done. You ok with this after a final pass?

mrshu commented 7 years ago

@zachthompson I just forgot a global reply here saying that I am OK with this.

bsstoner commented 7 years ago

👍 I'm good, assuming it's been thoroughly tested.