app_server flag fixes (patch)

[rogueeve]

Patch candidate for some app_server issues having to do with B_NO_SERVER_SIDE_WINDOW_MODIFIERS, as discussed on the haiku-dev mailing list. The second file is a program that can be used to test the changes. Changeset is:

• Fixed B_NO_SERVER_SIDE_WINDOW_MODIFIERS and B_CLOSE_ON_ESCAPE window flags not working.

• Added B_VALID_WINDOW_FLAGS constant to Window.h and modified app_server's ValidWindowFlags() function, to eliminate the need for a list of flags to be in two places, and possibly avoid the above problem in the future.

• The same method unfortunately would not work for the WINDOW_FEEL and _LOOK constants, because they are not bitflags, so a warning was added to Window.h about the possible need to update app_server if they are changed (a bit of a todo, I guess)

• CTRL+ALT-click "server side window modifiers" now require exactly the CTRL+ALT shortcut to be down; CTRL+ALT+SHIFT for example is not counted.

• CTRL+ALT clicks are ignored and let through to the window if no special action would have been taken anyway; e.g. if the window is not movable.

[rogueeve]

program which tests the above functionality

[leavengood]

Hi Caitlin,

Thanks for the patch, there are several style violations and I'm going to be more strict now that you've gotten a few patches committed. I know it is a long read, but please be sure your code follows our ​Coding Guidelines. While we don't all agree on each and every guideline, we all do follow them :)

In this patch some problems are:

• lines 783-785, I think there are extra parenthesis and you moved the && operator to the end of the line. The operator should be at the beginning of the next line.
  

• line 806, same problem with the && operator.
  

• line 919 is now longer than 80 characters (just barely at 81, but we are strict.) Please put it back as it was.
  

When you make these changes please update the patch and leave a comment so I get notified.

[rogueeve]

Updated patch to correct styling violations.

[jackburton]

Thanks for your work! I only have one remark: since there is no need for window valid flags to be public, I'd avoid the B_VALID_WINDOW_FLAGS in Window.h, and use instead a kValidWindowFlags, keeping it private.

[rogueeve]

Yes you're right, and that was something I was concerned about as well. I found "headers/build/private/interface/WindowPrivate.h", which I assume would be the proper place for "kValidWindowFlags". However, that still necessitates the matching list of flags across two source files, possibly leading to someone adding another flag later and forgetting to also add it into WindowPrivate.h. If modifying the public presentation of the global <Window.h> isn't acceptable I could add an #ifdef around B_VALID_WINDOW_FLAGS such that it is only defined when included by the app server, but that seems pretty ugly. Or I could just add a warning comment about the need to update 2nd list, as was done with the FEEL and LOOK constants.

[stippi]

"Or I could just add a warning comment about the need to update 2nd list, as was done with the FEEL and LOOK constants."

Yes, that would be my approach as well.

[rogueeve]

same patch as above, but does not add B_VALID_WINDOW_FLAGS

[rogueeve]

Added another version of the patch which omits B_VALID_WINDOW_FLAGS in favor of just a warning.

[stippi]

I wonder why we even need IsValidWindowFlags() on the server side... The warnings in the header don't really belong there, but I have no better idea. Anyway, I applied your patch here and am going to test a bit. What I don't like so much is that the system modifiers can be used if the window is not movable, I didn't apply that part. That's just some weird mix-up. An application developer should consciously control this via the provided window flag.

[stippi]

Apparently, the important parts of this patch were commited, closing. Thanks again!