Opened 4 months ago

Closed 3 months ago

Last modified 3 months ago

#353 closed bug (invalid)

Setting MUIA_String_Contents triggers MUIA_String_EditHook

Reported by: Andreas Falkenhahn Owned by: Thore Böckelmann
Priority: normal Milestone: 5.0-2017R3
Component: String.mui Version: 5.0-2017R2
Severity: minor Keywords:
Cc: OS Platform: All
Blocked By: Blocking:
Release Notes:

Description

I've noticed that with the latest MUI version setting MUIA_String_Contents triggers MUIA_String_EditHook. This breaks my app because MUI 3.8 didn't behave that way. In 3.8, MUIA_String_EditHook was only called on user input, not on setting MUIA_String_Contents.

Attachments (1)

main.c (3.3 KB) - added by Andreas Falkenhahn 4 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 4 months ago by Thore Böckelmann

Why does it break your application? The application's own edit hook must check the EditOp field of the supplied SGWork structure and must perform the desired actions for the operations it supports. Unknown or unhandled operations must be ignored. When setting MUIA_String_Contents this is either EO_CLEAR (empty string contents) or EO_BIGCHANGE (any other string).

These are the corresponding "documentation" sections from intutition/sghook.h:

#define EO_BIGCHANGE   (0x000A) /* unused by Intuition          */
    /* complete or major change to the text, e.g. new string    */
...
#define EO_CLEAR       (0x000C)
    /* clear the string                                         */

Ok, it says "unused by Intuition" but not "forbidden".

comment:2 Changed 4 months ago by Andreas Falkenhahn

In Hollywood Designer there are two string gadgets next to each other, one for entering a width value and one for entering a height value. When you type into the width gadget, the value in the height gadget is updated as you type by simply applying the object's aspect ratio to calculate the height value from the width value. This is also done the other way round, i.e. when typing into the height gadget, the value in the width gadget is updated as you type. Of course, this involves setting MUIA_String_Contents from the edit hook. And this doesn't work any longer because setting MUIA_String_Contents now triggers the edit hook so we end up in a deadlock.

comment:3 Changed 4 months ago by Thore Böckelmann

This is exactly the behaviour that was implemented for #275. It has been in there since 20 month now. So this isn't really new. I really wonder why you did not notice this long before.

What I can do is to avoid calling the edit hook of an object recursively.

comment:4 Changed 4 months ago by Thore Böckelmann

Milestone: future release5.0-2017R3
Owner: set to Thore Böckelmann
Status: newassigned

comment:5 Changed 4 months ago by Thore Böckelmann

In 6007:

  • String.c: don't call the edit hook a second time in case the first call has not retured yet. This refs #353.

comment:6 Changed 4 months ago by Andreas Falkenhahn

This still doesn't solve the problem. I think the only way to fix this is to revert to the MUI 3.8 behaviour, i.e. setting MUIA_String_Contents mustn't trigger MUIA_String_EditHook at all. With your latest fix I now get the phenomenon that the string gadget you type in receives different keys than you pressed. This is because the width hook calls MUIA_String_Contents on the height hook and the height hook then modifies the width gadget again.

It's all rather complicated to explain so I'm attaching a little example to show you what's going on here. Please try the attached example. It works correctly on MUI 3.8 but not with the latest MUI 5.0.

You can already see it right after starting the program: On MUI 3.8 the width gadget contains "633" whereas on MUI 5.0 it contains "0". This is because MUIA_String_Contents triggers MUIA_String_EditHook on 5.0. As soon as you type in something, all hell will break loose on MUI 5.0 because of the different behaviour on 5.0. Just try it for yourself and see.

Changed 4 months ago by Andreas Falkenhahn

Attachment: main.c added

comment:7 Changed 4 months ago by Thore Böckelmann

First of all, your hook function definition is slightly wrong. The third parameter (in a1) is not a pointer to an Object, but a pointer to a structure starting with a ULONG defining the structure contents. Currently there is no real structure, but only the leading ULONG which contains either SGH_KEY or SGH_CLICK of which MUI uses SGH_KEY only.

Second your edit hook should check this ULONG value for being either SGH_KEY or SGH_CLICK. Then it should check sgw→EditOp and act appropriately instead of doing the same action for each and every edit operation. There is a reason why these definitons exist. If you correctly distinguish between single characters being entered/deleted (EO_INSERTCHAR, EO_DELFORWARD/BACKWARD) and a full content replacement (EO_BIGCHANGE) then your hook will work with both MUI3 and MUI5.

I modified your example to this:

HOOKPROTO(streditfunc, ULONG, struct SGWork *sgw, ULONG *msg)
{
  ULONG rc = 0;

  if(*msg == SGH_KEY)
  {
    APTR *hdata = (APTR *)hook->h_Data;
    Object *other = hdata[0];

    switch(sgw->EditOp)
    {
      case EO_CLEAR:
      case EO_INSERTCHAR:
      case EO_DELFORWARD:
      case EO_DELBACKWARD:
      {
        int size = sgw->LongInt;
        char tmpstr[256];
        double asprat = (double) 173 / (double) 633;

        if(hdata[1] == (APTR)1)
        {
          snprintf(tmpstr, sizeof(tmpstr), "%d", (int) ((double) size * asprat));
        }
        else
        {
          snprintf(tmpstr, sizeof(tmpstr), "%d", (int) ((double) size / asprat));
        }

        set(other, MUIA_String_Contents, tmpstr);
      }
      break;

      case EO_BIGCHANGE:
      {
        // don't do anything on a full content replacement
      }
      break;
    }

    // signal that we understood the command
    rc = ~0;
  }

  return rc;
}
MakeHook(stredithook, streditfunc);
struct Hook stredit[2];
APTR hookdata[2][2];

// pass the hook function a pointer to the "other" object and an indicator how to do its aspect ratio calculation
hookdata[0][0] = height; hookdata[0][1] = (APTR)0;
hookdata[1][0] = width;  hookdata[1][1] = (APTR)1;

InitHook(&stredit[0], stredithook, &hookdata[0]);
InitHook(&stredit[1], stredithook, &hookdata[1]);
set(width, MUIA_String_EditHook, &stredit[0]);
set(height, MUIA_String_EditHook, &stredit[1]);

comment:8 Changed 4 months ago by Andreas Falkenhahn

Thanks, it's working correctly now on both 3.8 and 5.0 though nobody can blame me for not having implemented this in the right way if MUI 3.8 only supported a very limited subset of the edit hook API.

comment:9 Changed 3 months ago by Thore Böckelmann

Priority: undecidednormal
Resolution: invalid
Status: assignedclosed

comment:10 Changed 3 months ago by Thore Böckelmann

In 6008:

  • String.c: reworked the edit hook handling to call the hook after each edit operation, especially after certain key presses like Backspace or Delete. This refs #353.
Note: See TracTickets for help on using tickets.