Markdown Here Upgrade Notification every time I open Chrome

[jhwarehouse]

Hi again Adam,
I didn't see a way to re-open issue 109 after adding this new info so I'm opening a new one so I apologize if you get this twice. I'm sorry to be the bearer of bad news but it's still happening for me with 2.9.1. I manually set the last-version to 2.9.1 and of course the issue went away again. Here's a screenshot that shows it was still showing 2.9.0 until I manually set it. I've also included a screenshot of the other extensions I have installed in case it's some kind of conflict. Let me know if you need anything else or have something you want me to try out for you.

[adam-p]

Unnnhhh.

This isn't the important question, but: Is the behaviour identical to before?

For reference, this is the offending code (probably): https://github.com/adam-p/markdown-here/blob/a572380b798e68a857ef98ee2f460b3f3ab360d8/src/chrome/backgroundscript.js#L17-41

First of all, I should explain this: chrome.storage.sync.set({'last-version': '2.9.1'}) is not identical to calling MDH's OptionsStore.set({'last-version': '2.9.1'}). The former is the example I gave in the last bug, but I subsequently re-created the animated give to use the latter. With the former, the value that's stored is the string 2.9.1, with the latter it's the string "2.9.1" (note the quotes) -- aka JSON.stringify('2.9.1'). You won't have broken anything if you used chrome.storage.sync directly, but it will affect some test results...

Secondly: Can you force yourself back into the bad state by using OptionsStore.set({'last-version': '2.9.0'})? I hope so.

Thirdly: In the background console can you run this -- preferably both in a broken and non-broken state:

chrome.storage.sync.get(function(rawOptions) {
  OptionsStore.get(function(mdhOptions) {

    // This is the actual installed extension version.
    var extVersion = chrome.app.getDetails().version;

    // This is the unprocessed option value stored in Chrome. Under normal
    // cases this should be '"2.9.1"' -- *with quotes*, since it's a JSON
    // representation of a string.
    var rawVersion = rawOptions['last-version'];

    // This is the option as reported by Markdown Here's options code. For our
    // purposes here, it's a JSON-parsed version of raw.
    var mdhVersion = mdhOptions['last-version'];

    console.log(
      extVersion, mdhVersion, rawVersion, 
      extVersion === mdhVersion,  // This needs to be true to prevent upgrade notification
      mdhVersion === rawVersion); // This is just for interest sake and may be true or false
  });
});

"Normal" output will look like this: 2.9.1 2.9.1 "2.9.1" true false. Output from your current non-broken state will probably look like this: 2.9.1 2.9.1 2.9.1 true true. Output from a broken state... I don't know.

Fourthly: You had Markdown Here installed for previous versions and this didn't happen to you? (Well, the upgrade notification is new, but the options page used to open on every upgrade.)

Fifthly: Do you use Chrome's browser sync? (I have a not-very-convincing theory that the last-version value might be getting synched between browsers... with the old value overwriting the new... but somehow when you manually set the option it fixes it... not a very good theory.)

Thanks.

[jhwarehouse]

Hi Adam,

Once again, thanks for being so responsive on this issue. Yes, I was able
to force myself back to a broken state using
OptionsStore.set({'last-version': '2.9.0'}) and
running OptionsStore.set({'last-version': '2.9.1'}) fixed it again. I use
Chrome on my laptop and on my android phone but I don't think I've got
anything installed to "sync" the browser pages between them and I wasn't
running Chrome on my phone at all during the issue. I've included a
screenshot of my sync settings in Chrome if that helps. Here's the output
of the command you asked me to send you in that broken state and then the
fixed state.
[image: Inline image 2]

[image: Inline image 1]

Thanks,
Chris

On Fri, Oct 11, 2013 at 10:04 AM, Adam Pritchard
notifications@github.comwrote:

Unnnhhh.

This isn't the important question, but: Is the behaviour identical to
before?

For reference, this is the offending code (probably):
https://github.com/adam-p/markdown-here/blob/a572380b798e68a857ef98ee2f460b3f3ab360d8/src/chrome/backgroundscript.js#L17-41

First of all, I should explain this: chrome.storage.sync.set({'last-version':
'2.9.1'}) is not identical to calling MDH's OptionsStore.set({'last-version':
'2.9.1'}). The former is the example I gave in the last bughttps://github.com//issues/109#issuecomment-25909344,
but I subsequently re-created the animated give to use the latter. With the
former, the value that's stored is the string 2.9.1, with the latter it's
the string "2.9.1" (note the quotes) -- aka JSON.stringify('2.9.1'). You
won't have broken anything if you used chrome.storage.sync directly, but
it will affect some test results...

Secondly: Can you force yourself back into the bad state by using OptionsStore.set({'last-version':
'2.9.0'})? I hope so.

Thirdly: In the background console can you run this -- preferably both in
a broken and non-broken state:

chrome.storage.sync.get(function(rawOptions) {
OptionsStore.get(function(mdhOptions) {

// This is the actual installed extension version.
var extVersion = chrome.app.getDetails().version;

// This is the unprocessed option value stored in Chrome. Under normal
// cases this should be '"2.9.1"' -- *with quotes*, since it's a JSON
// representation of a string.
var rawVersion = rawOptions['last-version'];

// This is the option as reported by Markdown Here's options code. For our
// purposes here, it's a JSON-parsed version of raw.
var mdhVersion = mdhOptions['last-version'];

console.log(
  extVersion, mdhVersion, rawVersion,
  extVersion === mdhVersion,  // This needs to be true to prevent upgrade notification
  mdhVersion === rawVersion); // This is just for interest sake and may be true or false

});});

"Normal" output will look like this: 2.9.1 2.9.1 "2.9.1" true false.
Output from your current non-broken state will probably look like this: 2.9.1
2.9.1 2.9.1 true true. Output from a broken state... I don't know.

Fourthly: You had Markdown Here installed for previous versions and this
didn't happen to you? (Well, the upgrade notification is new, but the
options page used to open on every upgrade.)

Fifthly: Do you use Chrome's browser sync? (I have a not-very-convincing
theory that the last-version value might be getting synched between
browsers... with the old value overwriting the new... but somehow when you
manually set the option it fixes it... not a very good theory.)

Thanks.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/119#issuecomment-26153635
.

[adam-p]

Thanks, Chris. I'm glad you can re-break yourself. ("glad")

But... Your images aren't coming through. If you edit directly on Github it might work better.

[jhwarehouse]

Hi Adam,
No problem at all. Just FYI, I did also try removing markdown here from chrome while in a broken state then re-adding it. The output showed 2.9.1 across the board. I then set the last-version back to 2.9.0 using the optionsstore method and was still able to break it, so at least it's repeatable ;)

[adam-p]

I was able to reproduce the problem by executing OptionsStore.set({ 'last-version': '2.9.0' }). That caused it to happen in Chrome 30 with sync turned on, but not in Chrome 32 (Canary) with sync turned off. (I didn't try turning sync on and off.)

I can't figure out why, but even though the OptionsStore.set({ 'last-version': appDetails.version }) call seems to succeed, the last-version value doesn't stick. Calling OptionsStore.get immediately after the set call (like, in the same code path) shows the (correct) most recent version, but calling it from the background script console shows the old version. And when the browser restarts it retrieves the old version and shows the notification again.

The fix (fd2c89a) is a cheap hack: setTimeout(upgradeCheck, 30000). Yep. For some reason doing the upgrade check -- and specifically the options value update -- well after the load event makes it okay.

I don't feel good about this, but it seems to work. Hopefully it works for @jhwarehouse and everyone, and not just me.

Closing this again!

[jhwarehouse]

Hi Adam,

I've switched back to the main chrome store version now but I wanted to let
you know that your fix seems to work for me. More to see if I could than
anything else I cloned the repo locally with subversion and loaded the
trunk version as an unpacked extension. I then verified that I could still
reproduce the "broken" state. I then changed to the development branch
that has that fix in it. The upgrade notification came up after about 30
seconds and once dismissed the last-version correctly changed to 2.9.1 and
I was not able to reproduce the "broken" state.

Thanks,
Chris

On Fri, Oct 11, 2013 at 1:27 PM, Adam Pritchard notifications@github.comwrote:

I was able to reproduce the problem by executing OptionsStore.set({
'last-version': '2.9.0' }). That caused it to happen in Chrome 30 with
sync turned on, but not in Chrome 32 (Canary) with sync turned off. (I
didn't try turning sync on and off.)

I can't figure out why, but even though the OptionsStore.set({
'last-version': appDetails.version }) call seems to succeed, the
last-version value doesn't stick. Calling OptionsStore.get immediately
after the set call (like, in the same code path) shows the (correct) most
recent version, but calling it from the background script console shows the
old version. And when the browser restarts it retrieves the old version and
shows the notification again.

The fix (fd2c89a fd2c89a)
is a cheap hack: setTimeout(upgradeCheck, 30000). Yep. For some reason
doing the upgrade check -- and specifically the options value update --
well after the load event makes it okay.

I don't feel good about this, but it seems to work. Hopefully it works for
@jhwarehouse https://github.com/jhwarehouse and everyone, and not just
me.

Closing this again!

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/119#issuecomment-26169733
.

[adam-p]

That's great. Thanks for testing it out.

I'm struggling a little with the possibility that there might be thousands of people who had the same problem but haven't spoken up. In that case, I should rush out a new release with this fix. But... probably there aren't? Surely more than 3 people would have chimed in? Surely.

Thanks again.

[jhwarehouse]

Hi Adam,

It never fails to amaze me what annoyances some people can ignore. If you
have another release coming up soon I might just wait, but if it's going to
be awhile I'd push it out to avoid people removing the extension in the
mean-time. Whichever way you go I really appreciate the amazing support
you've given us.

Thanks,
Chris

On Sat, Oct 12, 2013 at 10:52 AM, Adam Pritchard
notifications@github.comwrote:

That's great. Thanks for testing it out.

I'm struggling a little with the possibility that there might be thousands
of people who had the same problem but haven't spoken up. In that case, I
should rush out a new release with this fix. But... probably there aren't?
Surely more than 3 people would have chimed in? Surely.

Thanks again.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/119#issuecomment-26202459
.