Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#288 closed bug (fixed)

MUIA_Group_Forward partially misbehaving for MUIA_Disabled

Reported by: Chris Handley Owned by:
Priority: normal Milestone: 4.0-2016R1
Component: Group.mui Version: 4.0-2015R4
Severity: minor Keywords:
Cc: OS Platform: All
Blocked By: Blocking:
Release Notes:

MUIA_Group_Forward will be ignored (again) when setting MUIA_Disabled, because nested objects must not be enabled while any of their parent objects is disabled.

Description

Summary

For certain reasons (not relevant to this report) I had been disabling buttons (that contain an image) using MUIA_Group_Forward,MUI_FALSE . This appears to work when disabling the button (it appears fully ghosted), but when re-enabling the button only the very outer part is unghosted.

I suspect that MUIA_Group_Forward is ignored when disabling the button, but respected when re-enabling, thus causing the inner parts of the button to be ghosted but not unghosted.

Steps to reproduce

The code for the button looks something like this:

Child, myObject := GroupObject,
	MUIA_Background,MUII_ButtonBack, MUIA_Frame,MUIV_Frame_Button, MUIA_InputMode,MUIV_InputMode_RelVerify,
		Child, GroupObject, MUIA_Group_Horiz,MUI_TRUE,
			MUIA_Frame,MUIV_Frame_None,
			Child, DtpicObject, MUIA_Dtpic_Name, myPicPath, End,
			Child, FreeLabel(myLabel),
			Child, FreeHoriz,
		End,
	End

The code to (un)ghost the button looked like this:

SetAttrs(myObject, MUIA_Group_Forward,MUI_FALSE, MUIA_Disabled,ghosted, TAG_DONE)

First use that code with ghosted=MUI_TRUE, then use it again with ghosted=MUI_FALSE.

(P.S. If any code looks a little odd, it's because I've manually converted it from E to C.)

Expected results

Button is fully unghosted, the same as when it was initially created.

Actual results

The button is still ghosted, except for the very outer part.

Regression

Notes

This bug is present in at least MUI-4.0-2015R4-os4 & MUI-4.0-2015R3 (possibly earlier).

Attachments (3)

Class1 (21.3 KB) - added by Thore Böckelmann 4 years ago.
modifed Class1 demo
Class1.c (6.5 KB) - added by Thore Böckelmann 4 years ago.
source of modified Class1 demo
Class1c.e (7.5 KB) - added by Chris Handley 4 years ago.
PortablE example MUI code which replicates the problem

Download all attachments as: .zip

Change History (17)

comment:1 Changed 4 years ago by Chris Handley

You can currently get a precompiled program that demonstrates this MUI bug here:
http://cshandley.co.uk/portable/examples/Stacker.lha

(At some point I'll release a fixed version of PortablE, which does not use MUIA_Group_Forward, at which point the precompiled program won't be buggy anymore!)

comment:2 Changed 4 years ago by Thore Böckelmann

Why are you nesting two group objects? One would be enough.

Using MUIA_Group_Forward is unnecessary in this situation. Cycle objects in fact are groups consisting of an Image object and a Text object. If you want to disable a Cycle object just don't use MUIA_Group_Forward, but simply set MUIA_Disabled for the Cycle object itself and hence effectively for the whole group.

Finally, please refrain from using external links for examples. These will vanish more sooner than later and will be no longer available for later reference. It is absolutely no problem to attach arbitrary files here to a ticket directly. This ensures that the files do not vanish.

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

Component: undefinedGroup.mui
Milestone: future release4.0-2016R1
OS Platform: undefinedAll
Priority: undecidednormal

comment:4 Changed 4 years ago by Chris Handley

I don't think any of your objections invalidates this bug report?

I already said my usage of MUIA_Group_Forward was not relevant to this report. But if you MUST know, it was to work-around another bug in MUI, that now seems fixed. And I already said I was going to stop using MUIA_Group_Forward.

I don't remember why I was using two GroupObjects. Probably to try to achieve the layout I wanted, or possibly because it didn't look correct without it. I'll try removing it & see what happens…

I did not attach my precompiled example because (1) it does NOT contain MUI nor C code, but rather abstract E code that dynamically creates the MUI code, and so the source code is no use, (2) I didn't think it was necessary to explain the problem, but merely might be helpful in certain cases, and (3) it's frikkin huge (contains binaries for AROS, MOS & 68k, as well as OS4), and frankly I didn't want to waste your server space on all that crap.

comment:5 in reply to:  4 Changed 4 years ago by Thore Böckelmann

Replying to ChrisH:

I already said my usage of MUIA_Group_Forward was not relevant to this report. But if you MUST know, it was to work-around another bug in MUI, that now seems fixed. And I already said I was going to stop using MUIA_Group_Forward.

If MUIA_Group_Forward is not relevant, then why is it contained in the ticket's title?

I don't remember why I was using two GroupObjects. Probably to try to achieve the layout I wanted, or possibly because it didn't look correct without it. I'll try removing it & see what happens…

The outer group has no influence on the inner group's layout. I already tried this and there is no visual difference regarding the layout.

I did not attach my precompiled example because (1) it does NOT contain MUI nor C code, but rather abstract E code that dynamically creates the MUI code, and so the source code is no use, (2) I didn't think it was necessary to explain the problem, but merely might be helpful in certain cases, and (3) it's frikkin huge (contains binaries for AROS, MOS & 68k, as well as OS4), and frankly I didn't want to waste your server space on all that crap.

No need to care about our server space. We have plenty of space there. If you think the file is too big you could have attached only the relevant binaries, i.e. AmigaOS3 and AmigaOS4 only. The source is definitely of interest, no matter which language, because it shows how things are done. And the binaries are of interest, too, because they enable us to test the result without having to build them ourself from the source.

comment:6 Changed 4 years ago by Thore Böckelmann

Just one more note. Are you sure you are using MUIA_Group_Forward consistently in every SetAttrs() call to set MUIA_Disabled? Keep in mind that MUIA_Group_Forward=FALSE disables the recursive setting of the attributes. If you first call something like
SetAttrs(obj, MUIA_Disabled, TRUE, TAG_DONE);

and later on

SetAttrs(obj, MUIA_Group_Forward, FALSE, MUIA_Disabled, FALSE, TAG_DONE;

then of course the inner objects will stay disabled, because you forbid recursion, and hence only the outmost object will be enabled.

Last edited 4 years ago by Thore Böckelmann (previous) (diff)

Changed 4 years ago by Thore Böckelmann

Attachment: Class1 added

modifed Class1 demo

Changed 4 years ago by Thore Böckelmann

Attachment: Class1.c added

source of modified Class1 demo

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

Please take a look at the modified Class1 demo source. It implements a button based on a group just like you do it. Clicking "enable" or "disable" will set MUIA_Disabled accordingly with recursion disabled. For me this works perfectly. The binary is for AmigaOS3/m68k and should work on AmigaOS4 as well.

Last edited 4 years ago by Thore Böckelmann (previous) (diff)

comment:8 Changed 4 years ago by Thore Böckelmann

Any news on this issue?

comment:9 in reply to:  4 ; Changed 4 years ago by Chris Handley

Replying to ChrisH:

I don't remember why I was using two GroupObjects. Probably to try to achieve the layout I wanted, or possibly because it didn't look correct without it. I'll try removing it & see what happens…

I just tried removing the second GroupObject, and it makes NO difference to the reported bug.

@tboeckel:

Just one more note. Are you sure you are using MUIA_Group_Forward consistently in every SetAttrs() call to set MUIA_Disabled?

Yup. The (un)ghosting is always done using a single method call… HOWEVER the initial ghosting (MUI_TRUE) is done after the MUI object is created BUT before it is embedded in a Window object & displayed. *I suppose this might be what causes different behaviour, rather than the ghosting state itself.*

I will look at your Class1 demo source, and see if modifying as described above makes a difference.

If MUIA_Group_Forward is not relevant, then why is it contained in the ticket's title?

My *reason* for using MUIA_Group_Forward is not relevant. i.e. I had discovered that it wasn't actually needed (anymore), but I didn't think discussing why I used it in the first place would be productive. Someone might use it for a different reason, and could still run into this bug.

Changed 4 years ago by Chris Handley

Attachment: Class1c.e added

PortablE example MUI code which replicates the problem

comment:10 in reply to:  9 Changed 4 years ago by Chris Handley

Replying to ChrisH:

Yup. The (un)ghosting is always done using a single method call… HOWEVER the initial ghosting (MUI_TRUE) is done after the MUI object is created BUT before it is embedded in a Window object & displayed. *I suppose this might be what causes different behaviour, rather than the ghosting state itself.*

I will look at your Class1 demo source, and see if modifying as described above makes a difference.

@tboeckel
As you said, your original Class1 demo does NOT show the problem. I modified the PortablE version of Class1 to be the same as yours, as it too did not show the problem either (although it seems my rusty MUI coding added a new bug, as the buttons stop responding after a few uses).

However, I then modified my Class1 code to ghost the button before it is added to the Window, and (as I suspected above) the ghosting bug now manifests! I will accept that this is a little unusual way of creating a MUI interface…

comment:12 Changed 4 years ago by Thore Böckelmann

In 5165:

  • Group.c: MUIA_Group_Forward will be ignored (again) when setting MUIA_Disabled, because nested objects must not be enabled while any of their parent objects is disabled. This refs #288.

comment:13 Changed 4 years ago by Thore Böckelmann

Somehow I messed up tickets #288 and #290 and falsely added this comment to ticket #290:

I had a deeper look at the MUI source. During MUIM_Setup of Area.mui the disabled state of any object's parent object is propagated to the object itself. This means that if a group object is disabled initally all its children are disabled automatically, too. This is necessary, because possible eventhandlers will be skipped for disabled objects. If a group object is disabled one expects that its child objects will not react to any kind of input.

Modifying MUIA_Disabled with MUIA_Forward set to FALSE effectively disables this quite intuitive behaviour. However, this is something that I added myself and now I see that this change was definitely wrong.

Back to your modified example source. Although MUIA_Disabled is set to TRUE with forbidden recursion to leave the nested object enable this is implicitly reverted as soon as the window is opened, because during MUIM_Setup the group's MUIA_Disabled=TRUE is propagated to all nested objects. If you later modify MUIA_Disabled again with MUIA_Group_Forward=FALSE only the outmost group object will be affected. This is definitely wrong. I will revert this.

comment:14 Changed 4 years ago by Thore Böckelmann

Resolution: fixed
Status: newclosed

comment:15 Changed 4 years ago by Thore Böckelmann

Release Notes: modified (diff)
Note: See TracTickets for help on using tickets.