Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flow + radio + clear breaks shoes #391

Closed
dredknight opened this Issue Nov 25, 2017 · 11 comments

Comments

Projects
None yet
3 participants
@dredknight
Copy link
Contributor

dredknight commented Nov 25, 2017

I managed to break Shoes (3.3.5 r3080) again. If you launch the code below and click the button it will break. I found 2 fixes - Commenting the flow line + end line or commenting the radio line.

Shoes.app do
	def main slot, stuff
		slot.clear do
			stuff.each_with_index do |x, i|
				flow do                             ####### FIX 1 : COMMENT THIS LINE 
					radio :item;          ####### FIX 2 : COMMENT THIS LINE 
				end                                   ####### FIX 1 : COMMENT THIS LINE 
			end
		end
	end
	
	ITEMS = Array.new( 10, 1 )
	desktop = stack left: 20, top: 40, width: 200 #do
	main desktop, ITEMS
	button "Reset" do
		main  desktop, ITEMS
	end
end
@BackOrder

This comment has been minimized.

Copy link
Collaborator

BackOrder commented Nov 25, 2017

This is a curiosity indeed. Clicking on reset lit up all the radio buttons. Did you try this with other widgets?

FIX 2 is kinda funny. Doesn't make much sense. :)

By the way, I removed the ** because bold cannot be used in code.

@dredknight

This comment has been minimized.

Copy link
Contributor Author

dredknight commented Nov 25, 2017

What do you mean lit them up? When I click reset I get runtime error :/

image

I added fix 2 just to show that bug is related to the radio button. Once removed it is all fine.

@BackOrder

This comment has been minimized.

Copy link
Collaborator

BackOrder commented Nov 25, 2017

You should have specified earlier. Anyway, this is what I get after clicking on reset on FreeBSD:

image

@dredknight dredknight added the Windows label Nov 25, 2017

@dredknight

This comment has been minimized.

Copy link
Contributor Author

dredknight commented Nov 25, 2017

At least we know it is Windows related :)

@BackOrder

This comment has been minimized.

Copy link
Collaborator

BackOrder commented Nov 25, 2017

It's not supposed to lit up all the radios either! So it affects us all.

@BackOrder BackOrder removed the Windows label Nov 25, 2017

@dredknight

This comment has been minimized.

Copy link
Contributor Author

dredknight commented Nov 25, 2017

Agreed :)

@ccoupe

This comment has been minimized.

Copy link
Contributor

ccoupe commented Jan 16, 2019

There is a sequence of Gtk error messages after I push the button:

(shoes:24321): Gtk-CRITICAL **: gtk_radio_button_get_group: assertion 'GTK_IS_RADIO_BUTTON (radio_button)' failed

Critical is usually a serious problem worth fixing. I'm guessing that the problem is the radio group :items is not cleared/deleted

@dredknight

This comment has been minimized.

Copy link
Contributor Author

dredknight commented Jan 16, 2019

Can I do something to help you finding the root cause?

@ccoupe

This comment has been minimized.

Copy link
Contributor

ccoupe commented Jan 16, 2019

Can I do something to help you finding the root cause?

Not yet. The sample code above is very helpful - thank you. I suspect part of the problem is that the 'group' list is supposed to be at the app level but appears to be done at the canvas level. This is likely to be a very old bug in Shoes - 7 or 8 years old.

@ccoupe

This comment has been minimized.

Copy link
Contributor

ccoupe commented Jan 28, 2019

We do have a confusing spec. The way I interpret the Manual and the code, radio groups do not belong to a stack/flow - they belong to the App/Window. So, clearing the slot as done in the example will not/should not clear a radio group.

It's also true that shoes_canvas_clear() will attempt to clear radio groups - it probably shouldn't (and gtk complains when it does). For fun, there's also code being called that does nothing useful. This is going to take some thinking and possibly and api change to Shoes

ccoupe added a commit that referenced this issue Feb 5, 2019

@ccoupe

This comment has been minimized.

Copy link
Contributor

ccoupe commented Feb 5, 2019

I've got a partial fix for the problem - you can get a windows beta at the usual place if you really want one - there are other radio problems. Use bugs/bug391.rb to test. - use code 'run this' code in the manual to find more problems

I've always had problems with how Shoes does radio buttons and revisiting the code just enforces my option that it's a bodge. After my latest visit to the hospital, I don't have a lot of mental stamina. Getting better, but it's taking it's sweet time.

ccoupe added a commit that referenced this issue Feb 9, 2019

for #391 - fixes the reported bug.
* There are others (osx).
* May not be possibile to fix OSX to match manual
* manual needs many warnings.

ccoupe added a commit that referenced this issue Feb 9, 2019

ccoupe added a commit that referenced this issue Feb 15, 2019

on #391 and #6 - Gtk is working OK.
* still have some debugging printfs
* OSX is not and may never work correctly

ccoupe added a commit that referenced this issue Feb 17, 2019

for #391, #6 - OSX working
* needs cosmetic fix - needs button image

ccoupe added a commit that referenced this issue Feb 20, 2019

@ccoupe ccoupe closed this in b1942d3 Feb 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.