21

Previously - posts that were a mix of closed, deleted or locked use to show at least two of those banners - however, now it only seems to show one.

If a post is closed and deleted (doing a search for closed:1 deleted:1) it only seems to be that the deletion banner is shown while it use to show the close banner and the deletion banner. They're definitely closed according to the timeline and the "reopen" option being available - there's just not the closure reason.

Posts that meet closed:1 deleted:1 locked:1 don't seem to show the lock or close banner... On here for instance the results from closed:1 locked:1 don't look right. Again, according to the timeline, they're definitely closed and locked - just no reason as to why is shown.

It looks to be a network wide thing and not just an SO one.

  • Right, I also thought something was off. – Glorfindel Jan 10 '18 at 12:58
  • 4
    Channels, it's always Channels. – Shadow Wizard Jan 10 '18 at 13:00
  • 1
    @Shadow you mean icantblamecaching.com for a change? :p – Jon Clements Jan 10 '18 at 13:01
  • 2
    Yeah, this one isn't about caching... they now work full time on channels, making tons of changes, and... not always testing before publishing, as this bug proves. :) (or changing one thing, that breaks something which is totally unrelated) – Shadow Wizard Jan 10 '18 at 13:02
  • Shouldn't we close, lock, and delete this post in order to demonstrate the problem? – Josh Caswell Jan 10 '18 at 13:08
  • 2
    @JoshCaswell that will work, but FHRC are even cooler! i.stack.imgur.com/EflIU.png – Shadow Wizard Jan 10 '18 at 13:15
  • Shog is looking at this bug – gnat Jan 12 '18 at 15:11
  • 1
    Thanks for the heads up @gnat :) – Jon Clements Jan 12 '18 at 15:13
14
+50

I'm gonna make this a fairly long answer, for reasons that'll soon become apparent. But the short version is: this is a bug and hic sunt FlagsAttribute.

Now... We need some background.

How this is supposed to work

There are 9 distinct "special statuses" defined for questions:

  • Deleted
  • Merged
  • MigratedTo
  • MigratedFromRejected
  • Closed
  • Locked
  • MigratedFrom
  • Protected
  • HasLocalizedVersion

This list is in priority order: the most important status is at the top (Deleted). Zero or more of these statuses may apply to any given question. If exactly one applies, it shall be displayed somewhere under the question; if more than one applies, between 1 and n statuses shall be displayed under the question, where n is the number of applicable statuses.

Only following combinations of statuses are allowed:

  • Merged, MigratedFrom
  • MigratedFromRejected, Closed
  • Closed, Locked
  • Closed, MigratedFrom
  • Locked, MigratedFrom
  • MigratedFrom, Protected
  • Deleted, Merged
  • Deleted, MigratedTo
  • Deleted, MigratedFromRejected
  • Deleted, Closed
  • Deleted, Locked
  • Deleted, MigratedFrom
  • Deleted, Protected
  • Deleted, HasLocalizedVersion
  • Deleted, Merged, MigratedFrom
  • Deleted, MigratedFromRejected, Closed
  • Deleted, Closed, Locked
  • Deleted, Closed, MigratedFrom
  • Deleted, Locked, MigratedFrom
  • Deleted, MigratedFrom, Protected

In addition, HasLocalizedVersion may be combined with any other statuses, so for brevity I'm going to ignore it for the rest of this answer. (As the list above demonstrates, this is true for Deleted as well... But due to an implementation detail that will become important in a later section, I can't omit it).

When deciding which applicable statuses to display below a question, the highest-priority status or combination of statuses should be chosen.

Examples:

  • A question is Locked and then Migrated to another site. Only the Migrated status is shown.
  • A question is migrated from Super User to Stack Overflow, Protected, then Closed as a duplicate and Merged into another question. The highest-priority applicable combination is MigratedFrom+Merged, so only those two statuses are displayed.

How this was implemented

The system described above came into being about 4 years ago. Prior to that, the definition for which statuses should be displayed was much more... Ad-hoc. The initial implementation was altered a bit over the years, but the basic technique remained roughly the same...

Priority flags

We start off by defining the statuses as bit-patterns, with decreasing values representing decreasing priorities:

                    Deleted                        // 100000000 -- highest priority
                    Merged                         // 010000000
                    MigratedTo                     // 001000000
                    MigratedFromRejected           // 000100000
                    Closed                         // 000010000
                    Locked                         // 000001000
                    MigratedFrom                   // 000000100
                    Protected                      // 000000010
                    HasLocalizedVersion            // 000000001 

Then, we define the valid combinations as OR'd bit-patterns:

            Merged                | MigratedFrom   // 010000100
            MigratedFromRejected  | Closed         // 000110000
            Closed                | Locked         // 000011000
            Closed                | MigratedFrom   // 000010100
            Locked                | MigratedFrom   // 000001100
            MigratedFrom          | Protected      // 000000110

Deleted |           Merged                         // 110000000
Deleted |           MigratedTo                     // 101000000
Deleted |           MigratedFromRejected           // 100100000
Deleted |           Closed                         // 100010000
Deleted |           Locked                         // 100001000
Deleted |           MigratedFrom                   // 100000100
Deleted |           Protected                      // 100000010
Deleted |           HasLocalizedVersion            // 100000001 
Deleted |   Merged                | MigratedFrom   // 110000100
Deleted |   MigratedFromRejected  | Closed         // 100110000
Deleted |   Closed                | Locked         // 100011000
Deleted |   Closed                | MigratedFrom   // 100010100
Deleted |   Locked                | MigratedFrom   // 100001100
Deleted |   MigratedFrom          | Protected      // 100000110

Picking the best statuses

For a given question, determining which statuses to display involves 5 steps:

  1. Compile a list of applicable statuses (a list of values each representing a bit-pattern defined in the first listing above).
  2. Aggregate the list into a single value (by ORing each value in the list generated by step #1)
  3. Generate a list of all valid statuses and combinations of statuses that can be represented by the bits in the aggregate generated in step #2
  4. Extract the largest value from the list generated in step #3: this is the highest-priority combination of applicable statuses
  5. Filter the list generated in step #1, retaining only those values which can be represented in the value extracted in step #4. This is the list of statuses which will be displayed below the question.

Note: I got a bit hand-wavy with two of those steps... The actual test I'm implying in steps #3 and #5 is generally implemented like this: flag & aggregate == flag, which verifies that every active bit in the flag is also represented in the aggregate. Now, chances are that anyone reading this has encountered this sort of test thousands of times in their career, and are aware that there's a common shortcut of the form flag & aggregate != 0 (or if you're using C, plain ol' flag & aggregate). This short form is only valid when you aren't testing for combinations of flags! Since combinations are rather important here, this was implemented with the full test (actually, it was implemented using .NET's HasFlag() method, but that does effectively the same test).

This may be easier to understand with an example. Let's use this question, which happened to be the first result returned when I searched for closed:1 locked:1.

  1. The list of applicable statuses is: [Locked, Closed, Protected]. Or, [000001000, 000010000, 000000010]

  2. The aggregate of #1 is 000011010

  3. The matching statuses and combinations are: [Locked, Closed, Protected, Closed | Locked]. Or, [000001000, 000010000, 000000010, 000011000]

  4. The largest value in the list generated in step #3 is 000011000

  5. Filtering the list from step #1 according to which values can be matched in the value from step #3, we get [Locked, Closed], which are the statuses to be rendered.

How this broke

Of course, since this is a bug report it clearly doesn't work the way I described above... So, what changed?

Well, remember back about 30 pages when I tossed in that pointless paragraph about common optimizations for flag tests and how they don't work if you care about flag combinations? Weeeelll... Turns out it wasn't so pointless: both steps #3 and #5 got their calls to HasFlag(status) replaced with & status != 0.

And now that example question gets processed like this:

  1. The list of applicable statuses is: [Locked, Closed, Protected]. Or, [000001000, 000010000, 000000010]

  2. The aggregate of #1 is 000011010

  3. The matching statuses and combinations are: [Locked, Closed, Protected, MigratedFromRejected|Closed, Closed|Locked, Closed|MigratedFrom, Locked|MigratedFrom, MigratedFrom|Protected, Deleted|Closed, Deleted|Locked, Deleted|Protected, Deleted|MigratedFromRejected|Closed, Deleted|Closed|Locked, Deleted|Closed|MigratedFrom, Deleted|Locked|MigratedFrom, Deleted|MigratedFrom|Protected]. Or, [000001000, 000010000, 000000010, 000110000, 000011000, 000010100, 000001100, 000000110, 100010000, 100001000, 100000010, 100110000, 100011000, 100010100, 100001100, 100000110]

  4. The largest value in the list generated in step #3 is 100110000 (which represents Deleted|MigratedFromRejected|Closed).

  5. Filtering the list from step #1 according to which values can be matched in the value from step #3, we get... [Closed].

Which matches the behavior you've observed.

Recommended fix

Do the full flag test in steps #3 and #5 described above.

Update: Jarrod has now implemented this via the canonical aggregate & flag == flag form. This produces correct results while avoiding boxing, which was the original reason for removing the calls to HasFlag() - every little bit helps.

You should now observe the expected behavior when visiting questions with multiple applicable statuses.

Notes

  • I should probably confess that the spec described in the first section above - including the weird-ass treatment of Deleted - is my fault. I started out just wanting MigrationFromRejected to work properly, and ended up cobbling together a doc during/after a call when it became apparent that nobody really agreed on how any of this was supposed to work. That m0sa was able to do anything sane with it was to his credit, not mine.

  • Both the lists and aggregate values described in the implementation section are essentially implementations of sets - the former is an efficient way of creating or passing around a set when you know you're never going to add duplicates, while the latter is an efficient way of testing for set membership.

  • I spent way too much time scratching my head over this behavior this afternoon. Clearly it's been too many years since I wrote anything in C.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .