Bug 135617 - Set Line Spacing dropdown menu always outlines Spacing: 1
Summary: Set Line Spacing dropdown menu always outlines Spacing: 1
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
5.2.0.4 release
Hardware: All All
: medium trivial
Assignee: Caolán McNamara
URL:
Whiteboard: target:7.2.0 target:7.1.1
Keywords:
Depends on:
Blocks: Main-Menu Sidebar-Properties-Paragraph Paragraph-Line-Spacing Split-Group-Buttons-Line-Spacing
  Show dependency treegraph
 
Reported: 2020-08-10 17:30 UTC by Terrence Enger
Modified: 2021-02-10 08:24 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Terrence Enger 2020-08-10 17:30:00 UTC
STR

(1) Start Writer from command line.  In formatting toolbar, observe
    font "Liberation Serif" and size "12 pt".

(2) In Writer document area, type "A <enter> B <enter> C <enter> D
    <enter> E <enter> "<ctrl>+A".  Observe the space which this
    occupies vertically in the document area.

(3) In formatting toolbar, expand the icon "Set Line Spacing".
    Observe that "Spacing: 1" is outlined.

(4) Click option "Spacing: 1.15".  Program closes the dropdown menu.
    Observe that the space required vertically in the document area is
    increased.

(5) Again, expand the icon "Set Line Spacing".

    good : "Spacing: 1.15" is outlined.
    bad  : "Spacing: 1" is outlined.

(6) Click option "Spacing: 1.5".  Program closes the dropdown menu.
    Observe that the space required vertically in the document area is
    further increased.

(7) Again, expand the icon "Set Line Spacing".   

    good : "Spacing 1.5" is outlined.
    bad  : "Spacing: 1" is outlined.

(8) Click option "Spacing: 1".  Program closes the dropdown menu.
    Observe that the spacing required vertically in the document area
    reduces to what it was in step (2).

I made these observations in commit 28022cee (2020-08-09), built and
running on debian-buster.

The problem appears with both VCL gtk3 and gen.  For comparison,

(*) some other icons on the formatting toolbar are *not* similarly
    afflicted: "Font Color", "Highlight Color", "Toggle Bulleted List"
    (which offers a selection of bullet styles), "Toggle Numbered
    List" (which offers a selection of numbering styles).

(*) In Calc, icon "Border Color" correctly displays previously
    selected state.  To my surprise, some other icons on the Calc
    formatting toolbar do not display the previously selected state:
    "Font Color", "Background Color".

I am setting severity trivial.
Comment 1 Terrence Enger 2020-08-10 17:32:22 UTC
In bibisect-linux-64-5.3 running on debian-buster, I see with
SAL_USE_VCLPLUGIN=gtk3 that outlining out the wrong present state
starts with the same commit which first outlines any state.  With
SAL_USE_VCLPLUGIN=gen, outlining the wrong present state already
happens in version oldest.

So, not obviously a regression.

As bugzilla does not offer 5.2.0.0.alpha1+, I am setting version
5.2.0.4 release.
Comment 2 Dieter 2020-08-16 15:54:24 UTC
(In reply to Terrence Enger from comment #0)
> STR
> 
> (1) Start Writer from command line.  In formatting toolbar, observe
>     font "Liberation Serif" and size "12 pt".
> 
> (2) In Writer document area, type "A <enter> B <enter> C <enter> D
>     <enter> E <enter> "<ctrl>+A".  Observe the space which this
>     occupies vertically in the document area.
> 
> (3) In formatting toolbar, expand the icon "Set Line Spacing".
>     Observe that "Spacing: 1" is outlined.
> 
> (4) Click option "Spacing: 1.15".  Program closes the dropdown menu.
>     Observe that the space required vertically in the document area is
>     increased.
> 
> (5) Again, expand the icon "Set Line Spacing".
> 
>     good : "Spacing: 1.15" is outlined.
>     bad  : "Spacing: 1" is outlined.

I confirm that

> 
> (6) Click option "Spacing: 1.5".  Program closes the dropdown menu.
>     Observe that the space required vertically in the document area is
>     further increased.

I can't confirm that.

> 
> (7) Again, expand the icon "Set Line Spacing".   
> 
>     good : "Spacing 1.5" is outlined.
>     bad  : "Spacing: 1" is outlined.

Same as after step 5


I used
Version: 7.0.0.3 (x64)
Build ID: 8061b3e9204bef6b321a21033174034a5e2ea88e
CPU threads: 4; OS: Windows 10.0 Build 19041; UI render: Skia/Raster; VCL: win
Locale: de-DE (de_DE); UI: en-GB
Calc: threaded

So I changed status to NEW and reduced bug summary to the first issue

Additional information: Same problem in the sidebar
Comment 3 Terrence Enger 2020-08-16 17:43:25 UTC
Thank you, Dieter, for your attention to this issue.

(In reply to Dieter from comment #2)
> (In reply to Terrence Enger from comment #0)
> 
> > 
> > (6) Click option "Spacing: 1.5".  Program closes the dropdown menu.
> >     Observe that the space required vertically in the document area is
> >     further increased.
> 
> I can't confirm that.

But this is just to set us up to see another example of the same
problem when we get to step (7).
Comment 4 Aron Budea 2020-12-23 14:09:36 UTC
In addition, with gtk3 VCL plugin, since the below commit no item is outlined in the line spacing dropdown for me:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=7e4b2d90f8b877bd954b7549e17c790fa05ea96a
author		Caolán McNamara <caolanm@redhat.com>	2020-01-07 20:01:32 +0000
committer	Caolán McNamara <caolanm@redhat.com>	2020-01-08 17:15:23 +0100

rework ParaLineSpacingPopup to be a PopupWindowController
Comment 5 Caolán McNamara 2021-01-18 16:12:18 UTC
wrt comments 1-3, the focus is just at its default location of first enabled widget. https://gerrit.libreoffice.org/c/core/+/109561 adds setting the focus to the widget matching the current state

wrt comment #4 https://gerrit.libreoffice.org/c/core/+/109560 is intended to address that by grabbing focus after visibility is set, which gives the expected result for the gtk case
Comment 6 Commit Notification 2021-01-19 09:08:32 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/eb4fe68e21868c066b68fb399f5ec0c207b0d4ef

Related: tdf#135617 grab focus after shown

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Commit Notification 2021-01-19 09:08:43 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5dea5dbb99b1925b2ecc2b2882d8cef30fe50d27

Resolves: tdf#135617 grab focus to widget matching the line spacing state

It will be available in 7.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2021-01-20 18:04:23 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

https://git.libreoffice.org/core/commit/d4c2a697a66fc3cf187fbd2d80e941a8c56275c2

Resolves: tdf#135617 grab focus to widget matching the line spacing state

It will be available in 7.1.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Dieter 2021-02-10 08:24:02 UTC
VERIFIED with

Version: 7.2.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 396c2ad2daad6fe6a11703d0ae1593929834afe2
CPU threads: 4; OS: Windows 10.0 Build 19042; UI render: default; VCL: win
Locale: de-DE (de_DE); UI: de-DE
Calc: threaded

Caolán, thanks for fixing it!