Incorrectly sizes new subwindows when the title bars are disable. 1.90-1 regression

[Julián Moreno Patiño]

From http://bugs.debian.org/846307

Hi.

I just noticed that a new terminator, is making some subwindows (created
via split of existing subwindow) zero size width or height, when using
disabled option 'show titlebar'.

It works correctly with titlebars enabled.

Everything was working fine in previous version.

[Egmont Koblinger (egmont-gmail)]

Yup, confirmed :(

Interestingly, if you press either Ctrl+Shift+E or Ctrl+Shift+O (but just one of these) multiple times after a fresh start of terminator, the first split is okay, the second is buggy, the third and later splits are okay again.

Also interestingly, in terminatorlib/titlebar.py update_visibility() line 187-ish if I change self.hide() to self.show() (so that the red title bar is shown) but leave self.label.hide() as is (that is, the label within is not shown) then it's still buggy. Showing the label too fixes the problem. (Note: 'label' is a complex EditableLabel object, not just a plain text label.)

[Stephen Boddy (stephen-j-boddy)]

Weird. I can't reproduce at all. Not disputing that there's an issue, I just can't replicate, so I'm at a loss how to troubleshoot it.

[Egmont Koblinger (egmont-gmail)]

Interestingly, a focus out repairs the layout.

[Egmont Koblinger (egmont-gmail)]

Changing the focus to a different pane also repairs the layout.

[Egmont Koblinger (egmont-gmail)]

As an experiment, I made the following changes in editablelabel.py:

- __init__(), line 48-ish: added a self._label.set_text("blah") just to get a default label (not necessary)

- set_text(), line 65-ish: removed the call to self._label.set_text(text).

Even with the titlebars switched on (the default) it becomes buggy with these changes.

Looks like setting the title label triggers a series of Gtk+ geometry recomputations which finally results in the correct layout; omitting this step optimizes away some recalculations and somehow certain Gtk+ events are then not acted upon immediately.

(At this point I cannot promise at all that I'll have time to continue investigating.)

[Egmont Koblinger (egmont-gmail)]

Right now I can only reproduce the bug if all of these 3 conditions are met:

- titlebar is disabled
- scrollbar is disabled by default
- current bzr (r1663) started up as "./terminator"

If I have any of these:

- titlebar enabled
- scrollbar enabled by default
- terminator-1.90 ubuntu zesty package started up as "terminator"

then it works as expected.

Which is quite weird because the original report is about the debian 1.90 package with (as far as I know) backported patches from bzr.

Could r1660 (bug 1645704) have accidentally changed the behavior?

Stephen, can you reproduce this bug using this information?

[Egmont Koblinger (egmont-gmail)]

If you open a second tab and then split then it's also broken.

That is: Splitting a tab is broken, splitting a pane is sometimes broken, splitting the entire window works correctly.

Splitting the entire window (that is, the first Ctrl+Shift+E or O keypress) flashes up "None None" in a gray bar at the top of the window for a short moment. That is, the titlebar (that's supposed to be disabled) is visible for a short while.

This back-n-forth toggling could be the one that accidentally triggers fixing the layout. Or not, since in the default "titlebar enabled" case it's not a back-n-forth, and it still works. Dunno.

My generic _feelings_ (without seeing what the actual bug is):

Probably a big cleanup along the lines of bug 1522542 is required.

Terminator is stuffed all over the place with "while Gtk.events_pending(): Gtk.main_iteration_do(False)". I believe this is a bad programming practice. In gtk2 it might have been a great workaround for plenty of things, in gtk3 this rather causes an uncontrollable mess.

The entire new layout should be imagined and then defined in one step, without giving the control back to gtk's main loop halfway that updates the UI for that partial solution and then noone knows how the rest of the story continues.

"while Gtk.events_pending(): Gtk.main_iteration_do(False)" is only okay after the entire new layout has been defined, and after this step we only do things that no longer influence the layout, e.g. focus a certain widget.

I have a strong feeling that a generic cleanup along these lines will lead to this bug magically vanishing.

Also there's too much code duplication (triplication -- is there such a word? :)) since window.py, notebook.py and paned.py pretty much define the same steps to be executed for these three kinds of Gtk objects. It would be great to clean them up, and e.g. have one single split_axis() that knows how to replace any of these widgets with a Paned. But that's probably too much for now.

[Stephen Boddy (stephen-j-boddy)]

Nope. Even using your three conditions, Egmont, it works as expected for me. I've even updated my system to 16.04 over the weekend (And that's a sore topic, BTW! What a mess!) so I'm on a newer version of GTK, 3.18 I believe. The only thing I can think of is that my crappy laptop is slow enough that I'm not running into some race condition.

[Egmont Koblinger (egmont-gmail)]

Yes it definitely smells like a race condition is involved, and those are hard to reproduce :(

Maybe you should break your laptop and buy a new one, as I did a month ago :D (just kidding)

[Emilio Pozuelo Monfort (pochu)]

Reverting commit r1620 fixes this bug for me.

The attached patch, which doesn't revert r1620 but instead sets the 'position-set' property only when actually setting the 'position' property, fixes it too.

Not sure what bug r1620 was trying to solve. I have read bug 1494979 and I can't reproduce the bug when going to r1619... but I'm not sure I understand it, or have the same config to reproduce.

Stephen, can you see if my patch doesn't regress the bug you fixed in bug 1494979 ?

[Stephen Boddy (stephen-j-boddy)]

Could someone who could see this issue give a verdict on Emilio's patch? It seems OK for me, but then I couldn't reproduce the issue anyway.

[Emilio Pozuelo Monfort (pochu)]

Anyone? :)

BTW it'd be nice if we could have a new 1.91 release. We have 1.90 in Debian right now and it'd be good to include all the bug fixes that are in the repo.

[Pavel Stehule (okbobcz)]

2017-01-20 10:04 GMT+01:00 Emilio Pozuelo Monfort <email address hidden>:

> Anyone? :)
>
> BTW it'd be nice if we could have a new 1.91 release. We have 1.90 in
> Debian right now and it'd be good to include all the bug fixes that are
> in the repo.
>

I can check it this weekend

Regards

Pavel

>
> --
> You received this bug notification because you are subscribed to a
> duplicate bug report (1648813).
> https://bugs.launchpad.net/bugs/1646257
>
> Title:
> Incorrectly sizes new subwindows when the title bars are disable.
> 1.90-1 regression
>
> Status in Terminator:
> Confirmed
>
> Bug description:
> From http://bugs.debian.org/846307
>
> Hi.
>
> I just noticed that a new terminator, is making some subwindows (created
> via split of existing subwindow) zero size width or height, when using
> disabled option 'show titlebar'.
>
> It works correctly with titlebars enabled.
>
> Everything was working fine in previous version.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/terminator/+bug/1646257/+subscriptions
>

[Pavel Stehule (okbobcz)]

2017-01-20 10:33 GMT+01:00 Pavel Stehule <email address hidden>:

>
>
> 2017-01-20 10:04 GMT+01:00 Emilio Pozuelo Monfort <email address hidden>:
>
>> Anyone? :)
>>
>> BTW it'd be nice if we could have a new 1.91 release. We have 1.90 in
>> Debian right now and it'd be good to include all the bug fixes that are
>> in the repo.
>>
>
> I can check it this weekend
>

This issue is fixed with this patch

Regards

Pavel

>
> Regards
>
> Pavel
>
>
>>
>> --
>> You received this bug notification because you are subscribed to a
>> duplicate bug report (1648813).
>> https://bugs.launchpad.net/bugs/1646257
>>
>> Title:
>> Incorrectly sizes new subwindows when the title bars are disable.
>> 1.90-1 regression
>>
>> Status in Terminator:
>> Confirmed
>>
>> Bug description:
>> From http://bugs.debian.org/846307
>>
>> Hi.
>>
>> I just noticed that a new terminator, is making some subwindows (created
>> via split of existing subwindow) zero size width or height, when using
>> disabled option 'show titlebar'.
>>
>> It works correctly with titlebars enabled.
>>
>> Everything was working fine in previous version.
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/terminator/+bug/1646257/+subscriptions
>>
>
>

[Stephen Boddy (stephen-j-boddy)]

Committed revision 1720.