[Bf-taskforce25] interface operator XXX's

[Brecht Van Lommel]

Hi Ton, others,

I went over the XXX's marked in the interface code after the changes and
tried to fix them. These are corner cases that I tried to work around
with suboptimal code, but I think that solving them can't be done just
in the interface code and most likely requires window manager changes.

1) button_activate_state:
	/* note we move the handler to the region when the block is open,
	 * so we don't interfere with the events as long as it's open */
	/* XXX (to brecht) removing this makes thing work proper */
	//WM_event_remove_modal_handler(&C->window->handlers, data->operator);
	//WM_event_add_modal_handler(C, &data->region->handlers, data->operator);

This workaround was added to solve an issue in overlapping menus. This
happens for example in this case:
- Open the View menu in the timeline window.
- Go to highlight Sub Menu and, the menu pops open.
- Now move diagonally downwards (not too fast) to an
  item in the lower part of the sub menu.
- The menu submenu will close right when you enter it.

This happened because the menus overlap 2 pixels or so, and both get the
event, so the lower menu will get its button hightlight. This should
probably be solved in the window manager somehow, such that overlapping
regions block events for regions below it. This should be optional
though, because it would give issues with tooltips.

2) button_activate_modal:
	data= op->customdata;
	/* XXX (for brecht) this happens when cancel() frees, and modal handler still runs */
	if(data==NULL)
	    return OPERATOR_FINISHED;

I don't think this can happen, it could happen indeed for the menu_block
operator since that can call cancel itself when it returns, but that
does not cancel the operator itself yet then. So I don't think this
check has to be here, unless there is a scenario I'm not thinking of,
but the equivalent check in menu_block_handle_modal should be kept
indeed.

3) menu_block_handle_modal:
	ar= bhandle->region;
	block= ar->uiblocks.first;

	/* XXX (for brecht) this happens when click on menu */
	if(block==NULL)
	    return OPERATOR_FINISHED;

It goes wrong earlier than this check, the region this is using is freed
already at this point. This operator is invoked manually in
ui_menu_block_create (interface_ops.c) when the menu block is created.
It should basically live as long as the menu block is open, but
currently it does not get stopped when the menu blocks' region is
removed. The modalops list was a way to attach this operator to the
region and have it automatically cancelled when the region is removed.
The problem is that the operator has to run at the window level since it
still works in the "safe zone" outside of the actual menu. Not sure how
this operator should be linked to the region, modalops made this
possible but there is probably a better way, but I'm not sure about the
solution here.

4) ui_menu_block_create:
	SWAP(ARegion*, C->region, ar); /* XXX 2.50 bad context swapping */
	WM_operator_invoke(C, WM_operatortype_find("ED_UI_OT_menu_block_handle"), NULL);
	SWAP(ARegion*, C->region, ar);

This is related to the above problem. Here it invokes that operator, but
it does it from a different context from the menu block region, since
the button opening the menu block is not in this region of course.

5) menu_block_handle_activate_button
    ot= WM_operatortype_find("ED_UI_OT_button_activate");

    // XXX  WM_operator_cancel(C, &butregion->modalops, ot);
    but->activateflag= activateflag;

    SWAP(ARegion*, C->region, butregion); /* XXX 2.50 bad state manipulation? */
    WM_operator_invoke(C, ot, event);
    SWAP(ARegion*, C->region, butregion);

The second XXX, setting the right regions is not needed anymore now,
since the region is set into the context when the operator runs (even
though its handler is at the window level), because it was invoked with
the region in the context.

The first XXX is because we need to cancel the previous highlighted
button before a new one can be highlighted. This is tied to the
button_activate operator. Maybe the highlight code has to be moved out
of that operator so no operator needs to be running for this. That's not
trivial, since you then need the other part of the operator to register
more complex keymaps rather than just handling these keys in
button_activate_modal (timer for tooltip or menu open, enter for action,
buttons, ctrl+c/v, ..).

Brecht.