Bug 164850 - Third column in insert bookmark dialog is not editable for SAL_USE_VCLPLUGIN=gen
Summary: Third column in insert bookmark dialog is not editable for SAL_USE_VCLPLUGIN=gen
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
7.5.3.2 release
Hardware: All All
: medium minor
Assignee: Not Assigned
URL:
Whiteboard: target:25.8.0
Keywords:
Depends on:
Blocks: Bookmarks
  Show dependency treegraph
 
Reported: 2025-01-25 13:17 UTC by Jan Rheinländer
Modified: 2025-03-23 06:35 UTC (History)
4 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 Jan Rheinländer 2025-01-25 13:17:24 UTC Comment hidden (obsolete)
Comment 1 Jan Rheinländer 2025-01-25 13:45:06 UTC Comment hidden (obsolete)
Comment 2 Jan Rheinländer 2025-01-25 16:39:43 UTC
So now I hunted it down.

Steps to reproduce:

export SAL_USE_VCLPLUGIN=gen

Run soffice, select some text (this is important) and insert a bookmark for it.

Unselect the text

Open the bookmarks dialog again. The bookmark appears in the list.

The third column "Text is editable, until you
a) Click the header on any column to change the sor torder
b) Try to re-arrange the headers by dragging

After doing a) or b), instead column 1 becomes editable.

IMHO the reason is that SvTabListBox::SetTabs(sal_uInt16, tools::Long, MapUnit) resets all tab flags to MYTABMASK and sets the flag of the first column to EDITABLE. It shouldn't do this because it is only modifying the tabs. There is another method SvTabListBox::SetTabs() which is used to initialize the tabs.

But I may be wrong.

This bug is connected with patch 180450 on gerrit, which I know think is only a partial solution of the problem.
Comment 3 Jim Raykowski 2025-01-26 03:49:04 UTC
Hi Jan,

With:

Version: 25.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 1b14b6cd69124c604edabce742e95c89053b3b12
CPU threads: 4; OS: Linux 5.15; UI render: default; VCL: x11
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

I was able to reproduce this bug.

I found the patch you submitted here:
https://gerrit.libreoffice.org/c/core/+/180450
Comment 4 Michael Weghorn 2025-01-29 20:57:24 UTC
(In reply to Jan Rheinländer from comment #2)
> IMHO the reason is that SvTabListBox::SetTabs(sal_uInt16, tools::Long,
> MapUnit) resets all tab flags to MYTABMASK and sets the flag of the first
> column to EDITABLE. It shouldn't do this because it is only modifying the
> tabs. There is another method SvTabListBox::SetTabs() which is used to
> initialize the tabs.
> 
> But I may be wrong.

That sounds very plausible and matches my first impression (but I'm not very familiar with that code, so can't give a definitive statement at this point either), s.a. comment in your Gerrit change https://gerrit.libreoffice.org/c/core/+/180450 .
Comment 5 Michael Weghorn 2025-01-29 21:16:45 UTC
Earlier commit that sounds somewhat related and mentions the issue:

    commit fe38553aef2121f358fb58e450ec69314aad851e
    Author: Michael Stahl
    Date:   Fri Jul 22 16:19:02 2022 +0200

        vcl: allow editing a column other than the first one in SvTabListBox
        
        There are 2 members mvTabList and aTabs in SvTabListBox and
        SvTreeListBox, and only aTabs is evaluated to determine which column
        should be edited when calling start_editing().
        
        This solution is probably not ideal; it would be better to have a
        parameter to select the column because one might want multiple editable
        columns with multiple buttons...
        
        Also, the native GTK implementation has the same bug (but there
        double-clicking the column works).
        
        Change-Id: I727792e635d3b610a2132ee5f4155e3ee610aaf2
        Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137362
        Tested-by: Jenkins
        Reviewed-by: Caolán McNamara

CC'ing Michael Stahl, in case he still remembers anything that might be useful input here.
Comment 6 Jan Rheinländer 2025-01-31 19:27:41 UTC
I changed the gerrit commit to reflect the discussion:
The problem seems to be solved by removing three lines from SetTab()

The solution is not complete, though. If you add a second call to set_column_editables() somewhere, e.g. in bookmark.cxx SwInsertBookmarkDlg::ModifyHdl at the end:

m_xBookmarksBox->set_column_editables({ false, false, true, false, false });

then the 2nd column is now editable instead of the third. Very confusing.

Steps to reproduce after inserting this line:
Insert two bookmarks
Open the dialog
Delete one of the bookmarks
Try to edit in the 2nd and 3rd columns
Comment 7 Jan Rheinländer 2025-02-01 06:37:46 UTC
> The solution is not complete, though. If you add a second call to
> set_column_editables() somewhere, e.g. in bookmark.cxx
> SwInsertBookmarkDlg::ModifyHdl at the end:
> 

Sorry, that's DeleteHdl() of course
Comment 8 Jim Raykowski 2025-02-01 18:52:02 UTC
(In reply to Jan Rheinländer from comment #7)
> > The solution is not complete, though. If you add a second call to
> > set_column_editables() somewhere, e.g. in bookmark.cxx
> > SwInsertBookmarkDlg::ModifyHdl at the end:
> > 
> 
> Sorry, that's DeleteHdl() of course

I didn't notice any difference when testing the patch and adding a second call to set_columns_editables at the end of ModifyHdl or DeleteHdl.

I have filed a separate bug report that may be what you are getting at. Please see Bug 164985.
Comment 9 Jan Rheinländer 2025-02-01 20:16:10 UTC
(In reply to Jim Raykowski from comment #8)
> (In reply to Jan Rheinländer from comment #7)
> > > The solution is not complete, though. If you add a second call to
> > > set_column_editables() somewhere, e.g. in bookmark.cxx
> > > SwInsertBookmarkDlg::ModifyHdl at the end:
> > > 
> > 
> > Sorry, that's DeleteHdl() of course
> 
> I didn't notice any difference when testing the patch and adding a second
> call to set_columns_editables at the end of ModifyHdl or DeleteHdl.
> 
> I have filed a separate bug report that may be what you are getting at.
> Please see Bug 164985.

Did you test with

export SAL_USE_VCLPLUGIN=gen

?
Comment 10 Jim Raykowski 2025-02-01 20:59:40 UTC
(In reply to Jan Rheinländer from comment #9)
> (In reply to Jim Raykowski from comment #8)
> > (In reply to Jan Rheinländer from comment #7)
> > > > The solution is not complete, though. If you add a second call to
> > > > set_column_editables() somewhere, e.g. in bookmark.cxx
> > > > SwInsertBookmarkDlg::ModifyHdl at the end:
> > > > 
> > > 
> > > Sorry, that's DeleteHdl() of course
> > 
> > I didn't notice any difference when testing the patch and adding a second
> > call to set_columns_editables at the end of ModifyHdl or DeleteHdl.
> > 
> > I have filed a separate bug report that may be what you are getting at.
> > Please see Bug 164985.
> 
> Did you test with
> 
> export SAL_USE_VCLPLUGIN=gen
> 
> ?

Yes I tested with SAL_USE_VCLPLUGIN=gen with the calls to set_columns_editables. Can you attach a video demonstration? It might help me understand what I may be doing wrong to not be able to reproduce this.
Comment 11 Jan Rheinländer 2025-02-02 08:02:51 UTC
(In reply to Jim Raykowski from comment #8)

> I didn't notice any difference when testing the patch and adding a second
> call to set_columns_editables at the end of ModifyHdl or DeleteHdl.
> 

Neither can I, now. Not sure whether that is reassuring or not...
Comment 12 Commit Notification 2025-03-22 22:40:14 UTC
Jan Rheinländer committed a patch related to this issue.
It has been pushed to "master":

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

tdf#164850 Avoid resetting edit flags when resetting column widths

It will be available in 25.8.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.