D21332: Okular Annotation: add line start/end style config only for Straight Line tool

[Tobias Deiminger]

tobiasdeiminger added inline comments.

INLINE COMMENTS

> annotationwidgets.cpp:499
>      {
>          m_useColor = new QCheckBox( i18n( "Inner color:" ), gb2 );
>          gridlay2->addWidget( m_useColor, 1, 0 );

Checking a polygons Inner color checkbox now causes segfault. It's because `LineAnnotationWidget::applyChanges` accesses `m_startStyleCombo` and `m_endStyleCombo` unconditionally. But in case of `m_lineType == 1` that QComboBoxes have never been constructed. Access should be guarded by `if ( m_lineType == 0 )`, and probably an additional `nullptr` check.

> annotationwidgets.cpp:528
> +
> +        for ( const QString &i: { i18n( " Square" ), i18n( " Circle" ), i18n( " Diamond" ), i18n( " Open Arrow" ), i18n( " Closed Arrow" ),
> +                        i18n( " None" ), i18n( " Butt" ), i18n( " Right Open Arrow" ), i18n( " Right Closed Arrow" ), i18n( "Slash" ) })

Are the leading whitespaces in " Square", " Circle", and so on, intentional?

> annotationwidgets.cpp:542
>      else if ( m_lineType == 1 )
>      {

Is this `else if ( m_lineType == 1 )` branch really necessary? Looks like the code within could just be moved to the previous `if ( m_lineType == 1 )` at L. 497.

REPOSITORY
  R223 Okular

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

To: knambiar, #okular
Cc: tobiasdeiminger, okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20190523/20f0fde2/attachment.html>