I have no issue with removing this specific line, but wouldn't it be better, if we removed the whole if statement? I would prefer to clean up this whole thing once and for all and not have some left over, that doesn't hurt as much as the current code, but is still not a clean solution.

I am not opposed to that. It will break us but we can come back and find a different solution when we have more time and/or just patch libs-gui when we build GNUstep.

Thank you very much for looking into this.

On a semi-related note, it would be nice if regular menus and context menus (right-click) would behave consistently in this regard. Thanks!

@probonopd You are right I missed checking context menus during my testing. However reverting 82717ee entirely does not fix this issue for context menus. That would be some other separate fix I think we would need to make and I am not sure if it would even relate to this code at this point.

@pkgdemon context menus were not working in a consistent way before, so I agree they can be handled in a separate PR.

@fredkiefer What is your final decision on this MR? I know it is affecting others so I am fine with whatever decision you make. I do need to get something scheduled on our backlog though if we are going to just revert my change (which again I am fine with).

I still prefer the full revert over just removing this one line. Why? When we remove this line there is only a break left that will exit the inner do/while loop. But that would happen anyway as the condition for the loop is not met. This means we have just four extra lines in the code left over without any behavioural change. And I prefer to rather have less code than more.
Given that analysis, I really don't understand how these two changes could end up in different results for you. So either my analysis is wrong or it should just be the same for you.

I have no issue with removing this specific line, but wouldn't it be better, if we removed the whole if statement? I would prefer to clean up this whole thing once and for all and not have some left over, that doesn't hurt as much as the current code, but is still not a clean solution.

After our discussion @fredkiefer , I performed some tests.

Status Quo: current code has horizontal menu broken (mac-style): click on the menu does not leave the menu open, an annoying bug.

Removing shouldFinish = YES is enough to have mac-style menus working again.
Of course removing the whole

    if (type == NSLeftMouseUp || type == NSRightMouseUp || type == NSOtherMouseUp)
      {
          shouldFinish = YES;
          break;  // Exit the loop to proceed to StopPeriodicEvents
      }

Fixes horizontal menus for me too, so the whole if should be removed.

The whole if-statement is not placebo, because the outer loop continues while it is a MouseEvent or it should finish, while shouldFinish is set YES if a MouseEvent is found - this is cause of the menu-click breakage.

I proposed an equivalent fix which is more comprehensive in #336