amiga-mui / texteditor

A well-known and used MUI custom class (TextEditor.mcc) which provides application programmers a textedit gadget. It supports features like word wrapping, soft styles (bold, italic, underline), a spell checking interface as well as an AREXX interface for scripting.
GNU Lesser General Public License v2.1
19 stars 4 forks source link

Double notification when selecting item from a context menu on TextEditor.mcc #28

Closed afalkenhahn closed 4 years ago

afalkenhahn commented 4 years ago

Not sure if this is a MUI or a TextEditor.mcc issue but when selecting an item from a context menu applied to TextEditor.mcc, there will be a double notification.

Test program attached. For comparison, if you access the context menu of the simple button, there'll be only one notification. If you access the context menu of the text editor gadget, there'll be two notifications. ctxtmenu.zip

tboeckel commented 4 years ago

Can you please provide a plain C program and/or its source code which exhibits exactly the same issue? A wrapper like Hollywood/RapaGUI/MUIRoyale might add further "features", which are beyond the scope of MUI or any custom classes. I can only resolve or investigate in possible bugs when they happen via the normal C language API, too.

tboeckel commented 4 years ago

Don't get me wrong. But if you want me to be able to reproduce or understand this issue I need to know how the mishehaving context menu is created and handled. The .hwp file cannot be loaded as plain text by a simple editor. As such such I cannot any details about how to reproduce this issue and how to investigate where and what might be done wrong.

afalkenhahn commented 4 years ago

Yeah, I know, but preparing C programs is a lot of work. Don't you have a debug version of MUI so that you can trace what's going on? The program is really small and the code for managing the context menu of the button and the text editor gadget is exactly the same.

tboeckel commented 4 years ago

There is no general catch-all debug code in MUI. If there never was any issue with a code path, why should there be debug code to catch an issue? There are so many ways to build a context menu (MUIA_ContextMenu, MUIA_ContextMenuHook, MUIM_ContextMenuBuild) and to handle the context menu choice (MUIM_ContextMenuChoice, MUIM_Application_ReturnID). How should I know how exactly you build your menu and how you handle the chosen item? I just built up a little demo application using MUIA_ContextMenu and MUIM_ContextMenuChoice for two subclasses of Area class and TextEditor class. For me there is no duplicate invokation of MUIM_ContextMenuChoice for any object. So from my point of view everything is done right by MUI and TextEditor class and the bug might be located in your wrapper code between MUI and Hollywood. But this is something that I don't know.

afalkenhahn commented 4 years ago

Ok, so here is a little C program that shows the issue. You have to listen to MUIA_ContextMenuTrigger using a hook and then the hook will be called twice. But only if you attach the context menu to the TextEditor gadget. If you attach the context menu to a button, it works fine and the hook is just called once.

#include <ctype.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

#include <exec/exec.h>
#include <exec/types.h>

#include <libraries/gadtools.h>
#include <libraries/mui.h>

#include <mui/TextEditor_mcc.h>

#include <proto/dos.h>
#include <proto/exec.h>
#include <proto/muimaster.h>
#include <proto/utility.h>

struct Library *MUIMasterBase = NULL;
struct MUIMasterIFace *IMUIMaster = NULL;

struct Library *IntuitionBase = NULL;
struct IntuitionIFace *IIntuition = NULL;

#ifdef __SASC

    #define MYREG(x) register __ ## x
    #define ASM    __asm
    #define SAVEDS __saveds

    #define FUNC_HOOK(ret, name, htype, hook, dtype, data, mtype, msg) \
        __asm __interrupt __saveds static ret name (register __a0 htype hook, register __a2 dtype data, register __a1 mtype msg)

    #define DISPATCHER_ENTRY(name)
    #define ENTRY(name) name
    #define HOOKENTRY(name) (HOOKFUNC)##name
    #define PROCGATE(name)
    #define GETNPENTRY(name) (ULONG)##name
    #define HOOK_INIT
    #define HOOK_EXIT

#elif defined(__amigaos4__)

    #define MYREG(x)
    #define ASM
    #define SAVEDS

    #define FUNC_HOOK(ret, name, htype, hook, dtype, data, mtype, msg) \
        static ret name (htype hook, dtype data, mtype msg)

    #define DISPATCHER_ENTRY(name)
    #define ENTRY(name) name
    #define HOOKENTRY(name) (HOOKFUNC)name
    #define PROCGATE(name)
    #define GETNPENTRY(name) (ULONG)name
    #define HOOK_INIT
    #define HOOK_EXIT
#endif

FUNC_HOOK(void, HookFunc, struct Hook *, hook, void *, a, void *, b)
{
    printf("Hook triggered\n");
}

int main(int argc, char *argv[])
{
    Object *win, *app, *bt, *bt2, *str;
    ULONG sigs = 0;
    ULONG id;
    APTR menu, menuitem;
    struct Hook hook;

    memset(&hook, 0, sizeof(struct Hook));

    hook.h_Entry = HOOKENTRY(HookFunc);

    IntuitionBase = (struct Library *) OpenLibrary("intuition.library", 0);
    IIntuition = (struct IntuitionIFace *) GetInterface(IntuitionBase, "main", 1, NULL);

    MUIMasterBase = OpenLibrary("muimaster.library", 0);
    IMUIMaster = (struct MUIMasterIFace *) GetInterface(MUIMasterBase, "main", 1, NULL);

    menu = MenustripObject,
        Child, MenuObjectT("Test"),
            Child, menuitem = MenuitemObject,
                MUIA_Menuitem_Title, "Test item",
            End,
        End,
    End;

    app = ApplicationObject,
        MUIA_Application_Title, "Foo",
        MUIA_Application_Base, "xxxxx",

        SubWindow, win = WindowObject,
            MUIA_Window_Title, "Bar",

            WindowContents, VGroup,

                Child, VGroup,
                    Child, str = TextEditorObject,
                        MUIA_TextEditor_Contents, "Hello World\nThis is a world\nHow are you",
                    End,

                    Child, bt = SimpleButton("Click me"),
                    Child, bt2 = SimpleButton("Click me 2"),
                End,
            End,
        End,
    End;

    set(str, MUIA_ContextMenu, menu);

    DoMethod(win, MUIM_Notify, MUIA_Window_CloseRequest, TRUE, app, 2, MUIM_Application_ReturnID, MUIV_Application_ReturnID_Quit);
    DoMethod(str, MUIM_Notify, MUIA_ContextMenuTrigger, MUIV_EveryTime, app, 3, MUIM_CallHook, &hook, MUIV_TriggerValue);

    set(win, MUIA_Window_Open, TRUE);

    while((id = DoMethod(app,MUIM_Application_NewInput,&sigs)) != MUIV_Application_ReturnID_Quit) {

        if(sigs) {
            sigs = Wait(sigs | SIGBREAKF_CTRL_C);
            if (sigs & SIGBREAKF_CTRL_C) break;
        }
    }

    MUI_DisposeObject(app);

    if(IIntuition) DropInterface((struct Interface *) IIntuition);
    if(IMUIMaster) DropInterface((struct Interface *) IMUIMaster);

    if(MUIMasterBase) CloseLibrary(MUIMasterBase);
    if(IntuitionBase) CloseLibrary(IntuitionBase);

    return 0;
} 
tboeckel commented 4 years ago

The fix is a quite ugly workaround for an essential design flaw of TextEditor class.

To be easily able to draw whenever it might be necessary this method is put on hold to be called again inside an artificial MUIM_Draw caused by the MUI_Redraw() call below. Unforturnately OM_SET is then called a second time from the dispatcher. If no special precaution is taken this will cause popup menus to be triggered twice due to the two OM_SET invokations. Hence try to skip the MUI_Redraw() call here when certain attributes are contained in the list.

tboeckel commented 4 years ago

te_trigger_fix.zip Test binaries for AmigaOS3 and AmigaOS4.