rkflx planned changes to this revision. rkflx added a comment.
Thanks for looking at this so quickly, Christoph! I believe you inspired me to find a good solution. I'll submit updates over the weekend. If you are interested in the backstory, read on: > I probably miss the big picture here. Why are these attributes needed? The lineWidth attribute, for example, looks like a thing from the past, where you could control the thickness of frames (CDE vs. Motif style). These are public API of `KSqueezedLabel` (via `QLabel`), thus any bug in them should be fixed. The main motivation was https://phabricator.kde.org/D6696#130719 for `setMargin`, but I don't see a reason why we should leave the rest buggy if the fix is just the same. > QLabel sends out a QEvent::ContentsRectChange event when the margins change or the frame style causes a size change. Could this be catched (using an event filter) instead of reimplementing the properties? My first reaction was: "Sadly there is no such event to be observed.", because that's what I concluded in https://phabricator.kde.org/D7164#134363 and observed again today (e.g. for `setMargin`, `KSqueezedTextLabel::resizeEvent` was never called, so the duplication of `squeezeTextToLabel` in `setMargin` was needed). Then I thought: "Surely, such behaviour must be a bug in Qt." (¹). After playing around some more, I now think the issue is just a misunderstanding in the autotests (see https://phabricator.kde.org/D7163): If I remove `squeezeTextToLabel` from `setMargin`, then label->setProperty(attribute.toLatin1().data(), amount); QVERIFY(label->isSqueezed()); fails for `ksqueezedtextlabelautotest testChrome:margin`, while label->setProperty(attribute.toLatin1().data(), amount); QTest::qWaitForWindowExposed(label); QVERIFY(label->isSqueezed()); passes. This means I can rip out all three property reimplementations (²) and we should be good to go (³). Next challenge: Finding a new reviewer for https://phabricator.kde.org/D6696. --- (¹) E.g. there's a difference between `setMargin` and `setContentsMargin`: The former is between text and (Q)frame aka "inside", the latter is between (Q)frame and the surrounding widgets aka "outside". I suspected both behaved differently regarding event emission. (²) Now not needed anymore, but I'm curious anyway: Do you know of any tool available to help check BC automatically? (³) I'll also change `contentsRect` to be public (it's a reimplementation of a public function, after all) and update the `@since` to 5.39. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D7164 To: rkflx, #frameworks, cfeck Cc: cfeck, dhaumann
