Meta Stack Exchange is a question and answer site for meta-discussion of the Stack Exchange family of Q&A websites. It only takes a minute to sign up.
Sign up to join this community
When checking the timeline of a post I noticed that "Post Closed" events are now attributed to Community, and the close voters were moved to the "comment" column. Before, all close voters were listed in the "by" column. The same happened for "Post Reopened".
However, the event was also turned into an "edited" event, with Community adding all the post content, which is a bit confusing.
Before (screenshot borrowed from this question):
Now (question):
This has been fixed!
This took a while to triage because it touched a ton of different functions I wasn't used to poking around in, including the post history and post history revision generation.
The source of the problem was a function that checked if the post history entry had any content changes. This is a function that effectively hasn't been touched since Atwood first committed it 12 years ago. The original function was:
public bool HasContent()
{
return !(string.IsNullOrEmpty(Tags) && string.IsNullOrEmpty(Title) && !string.IsNullOrEmpty(Body));
}
This function was then modified to add a new component involving checking for some new feature that we haven't shipped. It looked similar to this:
public bool HasContent()
{
return !(NewFeature != NewFeature.None && string.IsNullOrEmpty(Tags) && string.IsNullOrEmpty(Title) && !string.IsNullOrEmpty(Body));
}
This function is kinda hard to read though - applying De Morgan's law we can negate the entire statement to make it a little more readable:
public bool HasContent()
{
return NewFeature == NewFeature.None || !string.IsNullOrEmpty(Tags) || !string.IsNullOrEmpty(Title) || !string.IsNullOrEmpty(Body);
}
The bug here is now pretty clear - we consider anything that has the new feature set to none as having its content changed. Because NewFeature was an enum, we have the behavior of default(NewFeature) == 0 == NewFeature.None, which meant that all post history items had this value set to NewFeature.None. This meant that all post history items were now considered to have content.
There's a function that transforms the list of post history entries to revision entries, and it depended on this check. With this buggy behavior, it started doing unpredictable things like displaying extra revisions for reopens or showing weird things in the post's timeline. Lots of other things depend of this content check working, so I'm certain that other features on the site were impacted too.
The approximate period of time this bug was live was:
HasContent function, as well as adding tests for the dependencies of HasContent that were demonstrating the buggy behavior. This bug is something that automated tests can and should catch - we're just missing test coverage here.
This bug seems to be much wider in scope, affecting actual site functionality, and not only display.
Not only closing events are affected, but other events (locking, deleting, etc.) are also affected.
See the below screenshot of a question's revision history:
When actions by multiple people take place, the "edit" is attributed to the Community user. When a binding action takes place, the "edit" is attributed to whoever took the action.
Also, I'm unable to rollback to prior revisions from before a closing/reopening, (un)deleting, or (un)locking action took place - clicking the Rollback button on such a prior revision produces an error message "Unable to rollback". (It turns out I wasn't reproducing this correctly - I was able to rollback to a prior revision, just not those that happen to contain the same content or are the current revision with text.)
Finally, this bug breaks links to specific revisions on posts - as the revision numbers are changed, and the URLs only contain the revision number and not the ID, a link to, say, revision 5 which took place after a history event, will link to the wrong revision, as that revision is now numbered 6.
!= instead of ==. We won't be able to push this tonight because we need to make sure that fixing this doesn't have unintended consequences elsewhere, but we'll be able to get it fixed ASAP tomorrow.
Sep 14, 2021 at 23:34
!=instead of==. We won't be able to push this tonight because we need to make sure that fixing this doesn't have unintended consequences elsewhere, but we'll be able to get it fixed ASAP tomorrow. Thank you so much for reporting this bug.!=to==: $1.00. Cost of knowing which!=to change: $999.00."