Timing issues causes popup to shrink about feedly-notifier HOT 58 CLOSED

seems to work for me

from feedly-notifier.

@tveimo thanks for the reporting.

Which browser are you using?
Could you please check if it happens when all other extensions are disabled?
Have you tried to reinstall the extension?

from feedly-notifier.

I tried reinstalling, no change. Turning off the background tab setting fixes the problem (so far). Am running Chrome version 54.0.2840.71 (64-bit).

Haven't tried disabling all other extension yet. Am unsure what the background tab setting helps with? speed?

from feedly-notifier.

@tveimo it determines behavior when you click on an article link in the popup.
When the option is enabled an article will be opened in a background tab and the popup will remain opened.
When the option is disabled and you click on any article link, the article will be opened in an active (focused) tab and the popup will be closed.

Probably we need to rename the option. Do you have some suggestions?

from feedly-notifier.

"Open links to feed items in background tab" maybe?

from feedly-notifier.

Just tried restarting the browser with only the feedly extension enabled, and it has the same behaviour. Now I have also managed to get the issue also with the background tab option enabled, so I guess it's not related to that. I've put up a screenshot of what happens here; https://dl.dropboxusercontent.com/u/6299520/Screen%20Shot%202016-11-17%20at%209.42.39%20pm.png

from feedly-notifier.

Could you please right click on extension icon and hit "Inspect popup"?
Are there any errors in "Console" tab?

from feedly-notifier.

No errors that I can see, but am not sure if errors will show up there unless the console is already open? I'm seeing this problem much less often now after I starting fiddling with it though.

from feedly-notifier.

No errors that I can see, but am not sure if errors will show up there unless the console is already open?

If there are errors in popup scripts, you'll see them.

Could you also go to chrome://extensions/ , enable developer mode and press "background page" near the extension? It'll open the same console window but for background scripts.

from feedly-notifier.

I haven't been able to find any more indications on what is causing this, but I've seen the same behaviour sometimes with the gmail notifier popup, so I'm starting to suspect this might be a chrome issue, possibly related to timing. I'm in Australia, so maybe I'm seeing higher latency to some of the services used by these plugins, so am curious if inserting some debug timeouts in parts of the code might help determine if it's actually caused by timing issue.

from feedly-notifier.

It appears that the popup shrinks to the size it has while it is displaying the throbber. Would it be possible to test out just having the throbber shown in the center of the window at its normal size, without resize? If this is the cause, then maybe the resize happens too late.

from feedly-notifier.

I tried reverting back to version 2.7.2, to avoid the async popup loading, and it seems to have fixed my problems.

from feedly-notifier.

@tveimo thanks for the investigation. I'll try to reproduce the problem.

from feedly-notifier.

Got the same problem but only on MacOs. Next time i´ll try to analyse it. Perhaps there is a way to reproduce.

from feedly-notifier.

same problem on chrome 55 macOS sierra, here is the aspect :

I think it's linked to the openning animation. It begin small, grows, then get back to original small size.

from feedly-notifier.

@dragonito @DavidBabel
Thanks for the help.

@tveimo are you using mac too?

from feedly-notifier.

Yes also on mac. I see a similar problem with the gmail notifier sometimes.

from feedly-notifier.

@dragonito
I've noticed that you forked the repo. I've written "build" steps just in case.
https://github.com/olsh/Feedly-Notifier/blob/master/README.md#build

Sadly, I don't have a mac right now, so, any help is appreciated. :)

from feedly-notifier.

Can take a look. But i never tried work on browser extension, you will have to strongly review my code.

from feedly-notifier.

What is supposed to be the "sandbox" command in the readme ?

from feedly-notifier.

@DavidBabel thanks.
If sandbox is specified, the will be compiled a sandbox version of the extension which uses sandbox feedly site http://sandbox.feedly.com/i/welcome
I usually use it for testing purposes.

from feedly-notifier.

Oh ok, i get it, missing a grunt before "sandbox". Didn't get it was a build option, and not a command named "sandbox" which build it :p

from feedly-notifier.

Yeah, sorry, will fix it. Thanks.

from feedly-notifier.

#62 do the trick, but it's a workaround.

The exact scenario i noticed it the following :
1 - popup expand normaly
2 - just after expand : another event is received and redefine width to dummy values

The "2" does not append everytime, but often anyway. It's possible to click multiple times on the icon to get back a "correct" behaviour.

The "setTimeout" i added override the "2" behaviour just after it appends. It's not clean, but i did not find where the problem is created.

from feedly-notifier.

The dummy values you mention, aren't they the size of the window shown when the throbber is shown, before the window is populated with feed entries?

from feedly-notifier.

The size is this one : #59 (comment) . It's close to.

But sometime it does like that on my tests :

Since it seems to only append on MacOs, it can be a chrome problem. I let @olsh judges if this workaround is good enouth.

from feedly-notifier.

@DavidBabel
I did small refactoring of your PR. Could you please test if everything works as expected?

from feedly-notifier.

The issue come back sorry 😭

from feedly-notifier.

Reverted it back, please do the final check and I push it to store.
Thanks.

from feedly-notifier.

I tested it this two ways :

• spam click on button
• high CPU charge

Both rarelly occur the bug again. Consider to increase the 50ms timeout to 300ms. Works for me.

I expect a strange other behavious i did not notice before.
After the grows of the popup smoothly, it instant change from "width" to "expandedWidth" size.

from feedly-notifier.

@DavidBabel
I've pushed another commit, could you please check again?
I tried another approach to address the issue.

from feedly-notifier.

Hey, @olsh , the issue came back. Sorry again

The deploy effect i described here #59 (comment) actually disapear

from feedly-notifier.

@DavidBabel
Thanks, so, we need to set it to 50ms or 300ms?

from feedly-notifier.

To exclude bug when spam click the button and the behaviour when cpu charge is high, it's better. But still a workaround.

Sorry i can't do better, i imagine how frustrating it could be to not be able to test by yourself

from feedly-notifier.

@DavidBabel
Sorry, I did a mistake in my commit, could you please test again? :)
If it falls again, I'll revert to your commit and increase the interval for mac platform.

Sorry i can't do better, i imagine how frustrating it could be to not be able to test by yourself

No, everything is OK, without you I wouldn't solve the issue at all.
Thank you for your help! 👍

from feedly-notifier.

Thanks ^^.

On this new version, it works well. The only behaviour which still trigger the problem is a "rapid 3 clicks" on the icon. But i have to admit, we don't do that on current use.
The high CPU test does not trigger the problem anymore.

I think you got a very acceptable solution now, since it worked as expected in a very large number of case during common use. Well done.

from feedly-notifier.

@tveimo @dragonito
I've just published the fixed version(2.12.1) to the store it'll available soon. Could you please test it and confirm that everything works?

from feedly-notifier.

Hi,
The bug is still here for me in 2.12.1 on Chrome 54 MacOs Sierra. 😉
https://youtu.be/5qCZP72Niq4

from feedly-notifier.

same bug for me with 2.12.1. this is very strange

from feedly-notifier.

Would it be possible to rerelease the version with async popup loading disabled, until this issue is resolved?

from feedly-notifier.

I've disabled async loading for Mac users in 2.12.1. And it didn't help.
https://github.com/olsh/Feedly-Notifier/blob/master/src/scripts/popup.js#L187

from feedly-notifier.

Hey ! My patch was ugly but actually corrected the problem. I admit that's a workaround, but a workaround which correct the issue.

from feedly-notifier.

Yes, your fix works, but I don't like the idea with big timeout (300ms), in this way the popup has a lag on windows (and probably on linux too).
I could push a version with 50ms, but you said that the bug happens sometimes. So, we need to find a 100% fix without side effects.

@DavidBabel could you please check that this branch is executed on Mac?
https://github.com/olsh/Feedly-Notifier/blob/master/src/scripts/popup.js#L188

@DavidBabel could you please check, if disabling async renedering(remove all executeAsync calls and call all functions directly) resolves this issue completely?

Guys, sorry, if I'd have a Mac, I would fix the issue more quickly.

from feedly-notifier.

Yeah don't worry, we understand fully, it's already a great job to consider your issues. I will make your tests tomorrow and will keep you in touch for the results.

from feedly-notifier.

Actually the timeout is human perceptible only when bigger than 250ms. But anyway i understand your concern. But in our case, it's not a big deal, and i will tell you why.

The point is :

• Your popup is growing when the button is clicked.
• It takes between 700ms to 1s to be fully grown.
• the "bug" occurs during this grow.
  So the patch which resolve the bug is applied during the grows of the popup, making the bug totally disappearing, without any visual glitch for the user. (cause 300ms is included in the 700ms of growing)

As my patch was a bit ugly in the code, the behaviour was perfectly nice for the user experience.

Hope i explain it clear enough, my english is tired at friday 5pm.

I do the tests tomorrow and keep you informed.

from feedly-notifier.

did not find the time yet. soon :)

from feedly-notifier.

Since this seems to happen to the gmail notifier as well, it looks like its a chrome bug; https://bugs.chromium.org/p/chromium/issues/detail?id=307912

Workaround: "There is clearly a race condition since with timeout 100 this is happening consistently, with timeout 110 only sometimes while with timeout 150 or 50 never."

from feedly-notifier.

@tveimo thanks for the info! 👍
Well, we need to add these timeouts (only for mac). But someone should test it.

from feedly-notifier.

How do I run the extension with real data from the feedly server? I've generated a developer id / clientSecret but I am unable to actually get any feed entries shown in the popup, I only get the login button.

from feedly-notifier.

You can't, you should use this server with the test credentials.
https://sandbox.feedly.com/

from feedly-notifier.

By editing feedly.api.js? I tried running grunt with clientId=sandbox, the clientSecret I generated, but when running the extension, it seems to be able to login, then tries to load http://localhost/?code=Ayka.... If I manually edit the url to the sandbox one, or just feedly.com, I can get into feedly, but I cannot get any feed entries in the popup. It still shows the login button.

from feedly-notifier.

You don't need to edit anything manually.
Run the following command:
grunt sandbox --clientId=sandbox --clientSecret=OE12J47X2W5PEF7CKPGZ --browser chrome

Load the extension from the build directory and login. After that go to https://sandbox.feedly.com/ and add some feeds.

from feedly-notifier.

Ok, I'm still only getting a login button in the extension though, so am unable to test.

from feedly-notifier.

Sorry for the noise. I didn't realise the client secret expires after a while and that the one in the REAME.md file is too old, am now using OE12J47X2W5PEF7CKPGZ, which works.

Changing the timeout of the executeAsync() method in popup.js from 0 to 150 seems to fix the shrinking popup issues on my macbook pro;

function executeAsync(func) {
setTimeout(function () {
func();
}, 150);
}

Can we please have a public release with that change, possibly optioned for mac clients only?

from feedly-notifier.

@tveimo
Thanks for the info, could you please get the latest version and test it again?
I've added the timeout for mac here
https://github.com/olsh/Feedly-Notifier/blob/master/src/scripts/popup.js#L194-L201

from feedly-notifier.

I've just pushed a new version with the fix live. Please try it out.

from feedly-notifier.

Works for me too. Suggest closing as resolved for now.

from feedly-notifier.

Thanks guys!
Feel free to reopen it.

from feedly-notifier.