D7164: KSqueezedTextLabel: Respect indent, margin and frame width

[Henrik Fehlauer]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170906/fa787bf3/attachment-0001.html>