When the resize event was triggered only the content
was resized and the widget itself was kept with same
height. This results in either huge gap between other
widgets or text which is cut of and rendered behind
other widgets.
BUG: 412829
Details
Details
- Reviewers
dhaumann - Group Reviewers
Frameworks
Show a message in KMessageWidget and resize
the widget. I made an application to test this error:
https://invent.kde.org/jurajo/bug-412829-test-app
Diff Detail
Diff Detail
- Repository
- R236 KWidgetsAddons
- Branch
- more-proper-height (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 21346 Build 21364: arc lint + arc unit
Comment Actions
Since KTextEditor uses KMessageWidget extensively we have to make sure the unit test messagetest still passes with this change.
Comment Actions
OK, I will try to write a test for it.
This change looks like it lacks some things to make it work properly in some cases.
I am not sure if I am the right person to fiddle with this code (but it is broken...)
The problem which I am faxing is that sizeHint is not changed when I resize the window.
Comment Actions
I just tested with KTextEditor's messagetest and KTextEditor's usage of KMessageWidget in the top and bottom bar, and all this looks still good.
+1 from my side. Another review would be welcome, though :-) Anyone?
Comment Actions
No, my tests say no.
This does not update sizeHint which can cause some issue later.
It sure looks nice in my test app but it is wrong in the core.
This is not the right solution.
Comment Actions
That's same issue when file is open then externally modified then updated by kmessagewidget button cause a frame to not resize correct?
Comment Actions
In that case I withdraw my review for now. I can only speak for Kate that I could not find a regression with my tests. But if you find other regressions, then this is likely not correct.
PS: KMessageWidget is a very unfortunate widget in this sense: We try in the QWidgets world to have nice animations with contents that resizes correctly. To do the animations, we use fixed sizes, however, at the same time the host application's size constraints could change, but if KMessageWidget is still in its animation phase, the wrong old sizes are used for the animation etc... That said, KMessageWidget is likely fundamentally broken, and it needs very careful testing to make sure we don't introduce regressions. That's why I did the tests for Kate/KTextEditor. But of course, also my tests test just a very limited amount of our reald-world scenarios.
Comment Actions
It looks good.
But I wrote a test as you suggested which still fails.
The test creates parent widget and puts the kmesagewidget and qlistwidget in it in vertical order (as in my test app)
and than it resizes the parent widget and compares the height (the available macros in *autotest.cpp)
I will also upload this test.
Comment Actions
This is the test I used to check the validity of my change, it kind of always fails.
I only hope someone can fix this widget at least for KF6.
Comment Actions
it would, but I am not such great guru.
I tried few ways to change the behaviour but it was even worse,
and since this is used in many programs I do not want to break them.