Specified Sourcegraph URL gets clobbered by the background script on the new optionsPage
chrismwendt opened this issue · 6 comments
- Debug the background page and run
chrome.storage.sync.set({serverUrls:['http://localhost:3080'],sourcegraphURL:'https://sourcegraph.com'}) - Open the options page (the URL should be https://sourcegraph.com)
- Open https://github.com/gorilla/mux/blob/master/mux.go
- Refresh the options page (the URL is now http://localhost:3080)
The culprit is this section of the migration:
@KattMingMing Can you shed some light on the intended behavior? What do each of these two storage keys represent, and what's the difference? Since there's only 1 Sourcegraph URL now, maybe it would help to get rid of serverUrls and keep sourcegraphURL.
As an aside, it took me quite a while to find this. It would have helped a lot if addMigration reused set rather than calling chrome.storage.sync.set directly. PR created #59
Updated the description. I'll reproduce it here for email searchability:
The culprit is this section of the migration:
@KattMingMing Can you shed some light on the intended behavior? What do each of these two storage keys represent, and what's the difference? Since there's only 1 Sourcegraph URL now, maybe it would help to get rid of serverUrls and keep sourcegraphURL.
As an aside, it took me quite a while to find this. It would have helped a lot if addMigration reused set rather than calling chrome.storage.sync.set directly. TODO link to PR
serverUrls is deprecated - I should have made that more clear in my pervious PR. I'll update the documentation / remove other serverUrls usage.
remove('serverUrls') is necessary to prevent this from happening.
Actually, we should leave serverUrls alone until we completely remove that old options page, otherwise we'll break its behavior. When do you think it'll get removed?
Hmm, it's a bit concerning that the migration was shipped and breaks the old options page. When you load a page on a code host, the Server URLs list gets truncated to length 1, and all the logic for retries is gone. Although, this only affects users who had multiple Server URLs configured (probably not many users).
The release is available on:
Your semantic-release bot