D19256: [RFC] height or width of FrameSvgItem can be set to a negative number, handle it..

[Benjamin ROBIN]

benjaminrobin created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
benjaminrobin requested review of this revision.

REVISION SUMMARY
  As explained in the bug report this is maybe not the right fix. It's hide the 
true problem.
  In the case of the bug report, a QQmlJavaScriptExpression generate a value 
equal to "nan" which
  is then converted to an integer, which gives -2147483648
  
  Reference:
  
  - https://bugs.kde.org/show_bug.cgi?id=403978
  - https://bugs.kde.org/show_bug.cgi?id=404109

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D19256

AFFECTED FILES
  src/declarativeimports/core/framesvgitem.cpp

To: benjaminrobin
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19256: [RFC] height or width of FrameSvgItem can be set to a negative number, handle it..

[Benjamin ROBIN]

benjaminrobin added a reviewer: Plasma.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D19256

To: benjaminrobin, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19256: [RFC] height or width of FrameSvgItem can be set to a negative number, handle it..

[Benjamin ROBIN]

benjaminrobin edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D19256

To: benjaminrobin, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19256: [RFC] height or width of FrameSvgItem can be set to a negative number, handle it..

[Aleix Pol Gonzalez]

apol added a comment.


  +1 makes sense to me

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D19256

To: benjaminrobin, #plasma
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D19256: [RFC] height or width of FrameSvgItem can be set to a negative number, handle it..

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D19256

To: benjaminrobin, #plasma
Cc: ngraham, apol, kde-frameworks-devel, michaelh, bruns

D19256: [RFC] height or width of FrameSvgItem can be set to a negative number, handle it..

[Nathaniel Graham]

ngraham added a comment.


  Why is this number negative though? That seems wrong. The bug report seems to 
indicate that Qt is returning a garbage value: 
https://bugreports.qt.io/browse/QTBUG-73691
  
  There's even a patch: https://codereview.qt-project.org/#/c/254226/
  
  Feels like we should close this as well as the bug report and focus on the Qt 
fix (assuming that's the right approach).

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D19256

To: benjaminrobin, #plasma
Cc: ngraham, apol, kde-frameworks-devel, michaelh, bruns

D19256: [RFC] height or width of FrameSvgItem can be set to a negative number, handle it..

[Benjamin ROBIN]

benjaminrobin added a comment.


  In D19256#418577 , @ngraham wrote:
  
  > Feels like we should close this as well as the bug report ...
  
  
  I am agree with you, but this patch still fix 2 very minor issues :
  
  - `resizeFrame(QSize(` was used instead of `resizeFrame(QSizeF(`
  - In the creation of the variable `QSize frameSize` the `width()` and 
`height()` were implicitly converted from qreal to int (QtCreator is not happy 
with it)
  
  So I can update this patch to only reflect these 2 minors changes, or forget 
them...

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D19256

To: benjaminrobin, #plasma
Cc: ngraham, apol, kde-frameworks-devel, michaelh, bruns

D19256: [RFC] height or width of FrameSvgItem can be set to a negative number, handle it..

[Nathaniel Graham]

ngraham added a comment.


  Updating the patch to only do that makes sense, but putting this 
minimum-value-is-always-zero guard in there too may also make sense just as a 
safety valve. I'll leave it up to the #plasma 
 folks at this point.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D19256

To: benjaminrobin, #plasma
Cc: ngraham, apol, kde-frameworks-devel, michaelh, bruns