Closed Gustavo-Hilario closed 2 years ago
Another report here: 4458333-zd
+1 on 4457082-zd-woothemes They have a Simple site (Free plan).
Another Reblog button missing here: 32753891-HC
Another report of Reblog missing in #31608113-HC and pushed follow-up to #4460447-zen
This and #58089 may possibly be related - the Reblog option doesn't show up in the reader either.
Another report here: https://wordpress.com/forums/topic/reblog-button-25/
have a fix diff ready for review D70189-code
One more report on 32772744-hc, which is moved to 4461575-zen.
Also reported in #4460865-zen
fixed with D70189-code users will have to open in a new tab and reload to see the reblog button due to caching
Hi @roo2 We have a report of this issue again where the Reblog button appears on the page when it is first loaded, but then disappears when the page is refreshed. The site is using a retired theme, so I wasn't sure if the fix would have applied to retired themes. Could you take a look to see if this instance is related to this issue/fix?
Reported here: 32937406-hc Follow up: 4624525-zen
I did some more testing on this and found the issue is not specific to the retired theme, Twenty Ten. I tested on Twenty Eleven, Twenty Sixteen, and Twenty Twenty-one, and can recreate the issue on all of them.
Steps to recreate the issue:
Sometimes the Reblog button appears at first view and disappears after refreshing a few times. Sometimes it doesn't appear at first view and then appears after refreshing a few times. It continues to appear and disappear with refreshing at random intervals.
thanks for reopening this @ccwalburn I've reproduced this and am trying to work out what's happened. It does appear to be failing to load intermittently.
It looks like it was It looks like the change D71144-code fixed a race condition related to the like button but introduced this race condition which is causing the reblog button to sometimes not be shown.
When I reverted the diff in my sandbox, I found that this issue no longer occurred ✅
My next step is to properly grok the change and work out how we can fix the original race condition without introducing this problem!
cc @lezama, @sgomes
Race condition bugs are the worst 😞 Let me know if you need any help grokking my fix or working on a new one, @roo2!
thanks @sgomes , I had a look at it today but couldn't quite crack it. What I know so far is:
When hard reloading the page, or loading it for the first time, the batchRequest
's success function ( defined in likes-rest-nojquery.js:1137
) is called before updatePostFeedback
. This results in reblog_path
being set in this.cache
( likes-rest-nojquery.js:925
)👍
What happens when the page is reloaded however, is that for some reason (why??) batchRequest's
success callback is not called until after updatePostFeedback
. Therefore at the time that updatePostFeedback
is called reblog_path
has not yet been added to this.cache
👎
I've double checked that if I revert D71144-code in my sandbox, hard and soft reloading both cause the events to fire in the correct order. 👍
But I can't yet work out how the changes in D71144-code would seem to cause the batchRequest
s success
callback to be delayed on a soft reload??
Anyway, if that makes sense and rings any bells for you, then please see if you can work it out, otherwise I'll take another look in my tomorrow!
Thank you, @roo2 ! You did a lot of excellent initial work there 🙂
The hard reload appears to be a red herring; in my testing, I was able to reproduce the problem even with empty cache + hard reloads. There's something else at play; it's a tricky problem, but I'll attempt to explain.
The issue appears to happen when the response to the initial batch request arrives after the loadLikeWidget
message is received. The loadLikeWidget
message triggers updatePostFeedback
, which needs reblog_path
in this.cache
; and it only gets added to the cache when the response to the initial batch is received. This means that there is a strict dependence on handling the batch responses first, before handling the loadLikeWidget
response.
Now, the reason this worked before is because there used to be a single batch at a time, and any new requests waited for it. And experimentally, with the old code, that is indeed what happens when the loadLikeWidget
message arrives before the initial batch: instead of a new request getting made immediately, we instead wait until the initial batch is done.
The problem with this approach is that it assumed that there was a single post, and therefore there would be a single initial batch. This is not the case in e.g. indices with multiple posts, so the batches for each post could clobber each other, which was the race condition that was fixed for likes. So my fix was to make batches independent of each other, with only the requests within them being aware of their existence. Which does not account for the reblog use-case 😕
So, it seems that the correct fix would not be to have batches as entirely separate scopes, but rather to have a list of batches with their post ID, and to have independent requests check against the batch for their post. I'll prepare that fix in wpcom and add you in as a reviewer, @roo2!
Fix at D72467-code.
Nice work @sgomes I had some minor feedback but it looks really good and solves the issue perfectly 👍
The above change has been deployed to production as r237627-wpcom, with a cache-busting fix follow-up in r237629-wpcom. Please give it a try when you get a chance!
I tested the posts where this issue was reported and the Reblog button is staying put. Thanks @roo2 and @sgomes ! 4669026-zen
Quick summary
Simple sites with the
reblog
button enabled are not showing it.Steps to reproduce
What you expected to happen
I'd expect to see the
reblog
button there.What actually happened
The
reblog
button is missing.Context
Reports here: 4457709-zd-woothemes and 27653118-hc
Operating System
No response
Browser
No response
Simple, Atomic or both?
Simple
Theme-specific issue?
No response
Other notes
No response
Reproducibility
Consistent
Severity
Most (> 50%)
Available workarounds?
No response
Workaround details
No response