In order to keep the issues separate (for now), I intentionally did not address the work begun in D10246 or tracked at https://git.reviewboard.kde.org/r/127054/

Once this portion is reviewed and any changes are addressed, I can continue on with the related work.

Yeah, let's focus on Find first.

However, Ctrl+F to focus the Find bar now does not work anymore, it closes the bar instead while it shouldn't (see D10246 for discussion).

In D12094#243881, @rkflx wrote:

However, Ctrl+F to focus the Find bar now does not work anymore, it closes the bar instead while it shouldn't (see D10246 for discussion).

Is there still consensus on what @ngraham wrote in D10246?

Henrik is right. Perhaps a sane compromise is for the button to be a toggle, but for the keyboard shortcut to open and focus on the text field, but not close.

I think I can decouple these two actions. Let me see what I can come up with.

Nice, you seem to have solved the problem where the other patch got stuck.

Patch working fine for me now in a short test. No time to look at the code though, sorry.

Behaviorally, I cannot find any flaws for new users/installations. The button toggles on click, and changes its state appropriately in all circumstances. The keyboard shortcut focuses the list, and the escape key closes it.

However, I noticed a bug for existing installations: now the toolbar item list has two Find items in it, I think because you renamed the edit_find action to toggle_find in dolphinui.rc:

...And the old one doesn't have the new toggle behavior. So existing users with the Find button in their toolbars won't actually get the feature.

Is there a way you can not rename edit_find?

In D12094#244026, @ngraham wrote:

Is there a way you can not rename edit_find?

It appears that edit_find is one of the standardized menu entries defined by the KActionMenu system. Any action using one of the reserved names gets automatically pulled into the proper menu. If I had left it as edit_find, the result would be two entries in the Edit menu - one for the "classic" action and one for the new toggle action.

I don't believe there's any getting around having two entries in the available actions for toolbar/menu editing. Perhaps one could be given a different label and/or icon? It's awkward because they're both "find"-related actions, but have to be implemented separately.

In fact, why do we need two of these to implement the feature?

In D12094#244048, @ngraham wrote:

In fact, why do we need two of these to implement the feature?

Because they're separate actions, as seen by the KActionMenu system. One item (toggle_find) operates the taskbar button, which toggles the search status for a view on and off. The second item (open_find) only sets the search mode to true, and in the process, forces the focus to the search input bar. As we want a keyboard shortcut to act differently than the taskbar button (they were originally linked), they need to be separated.

The taskbar button is set up as a KToggleAction, which means it changes state with each activation (the visual state can also be changed manually, without triggering the linked action). The keyboard shortcut is a QAction, which does not have any toggle functionality. It just triggers a block of code when executed; it's got no concept of "on" or "off".

Either operation, be it a KToggleAction or a QAction, gets identified internally by a specific "action code" (for lack of a better term). This is where we get toggle_find and open_find as two distinct items. They're both in the application's overall set of controls, aliased as actionCollection() in this case.

The two items can absolutely have different labels or icons. At the moment, they're identical, and that's probably not ideal. How to differentiate between them is something for more discussion.

In D12094#244026, @ngraham wrote:

the toolbar item list has two Find items in it

Hm, I don't think having two Find entries each in Configure Toolbars and in Configure Shortcuts is okay, in particular for the shortcuts this will cause some confusion.

Can we find another solution which avoids adding open_find? Perhaps keep KToggleAction for most of the functionality, and only override keyboard handling to focus instead of close? In the end, the keyboard shortcut displayed for the menu entry and configurable in the dialog should also actually focus the Find bar.

I'm going to need some time to keep digging into this. I understand what is being requested; I have to see how to make it work within the existing menu/toolbar framework.

I get a crash when I try to open the control menu:

Thread 1 (Thread 0x7f6621f67d80 (LWP 32219)):
[KCrash Handler]
#6  0x00007f661936d860 in raise () from /usr/lib/libc.so.6
#7  0x00007f661936eec9 in abort () from /usr/lib/libc.so.6
#8  0x00007f661a059354 in QMessageLogger::fatal(char const*, ...) const () from /usr/lib/libQt5Core.so.5
#9  0x00007f661a058b35 in qt_assert(char const*, char const*, int) () from /usr/lib/libQt5Core.so.5
#10 0x00007f6621accf24 in DolphinMainWindow::addActionToMenu (this=0x55d0a3606770, action=0x0, menu=0x55d0a37b8c90) at ../src/dolphinmainwindow.cpp:1500
#11 0x00007f6621acbf92 in DolphinMainWindow::updateControlMenu (this=0x55d0a3606770) at ../src/dolphinmainwindow.cpp:858
#12 0x00007f6621ad916b in QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (DolphinMainWindow::*)()>::call(void (DolphinMainWindow::*)(), DolphinMainWindow*, void**) (f=(void (DolphinMainWindow::*)(DolphinMainWindow * const)) 0x7f6621acbe50 <DolphinMainWindow::updateControlMenu()>, o=0x55d0a3606770, arg=0x7ffcd947faf0) at /usr/include/qt/QtCore/qobjectdefs_impl.h:134
#13 0x00007f6621ad90d3 in QtPrivate::FunctionPointer<void (DolphinMainWindow::*)()>::call<QtPrivate::List<>, void>(void (DolphinMainWindow::*)(), DolphinMainWindow*, void**) (f=(void (DolphinMainWindow::*)(DolphinMainWindow * const)) 0x7f6621acbe50 <DolphinMainWindow::updateControlMenu()>, o=0x55d0a3606770, arg=0x7ffcd947faf0) at /usr/include/qt/QtCore/qobjectdefs_impl.h:167
#14 0x00007f6621ad8ff6 in QtPrivate::QSlotObject<void (DolphinMainWindow::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (which=1, this_=0x55d0a3b91690, r=0x55d0a3606770, a=0x7ffcd947faf0, ret=0x0) at /usr/include/qt/QtCore/qobjectdefs_impl.h:396
#15 0x00007f661a28b8ef in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib/libQt5Core.so.5
#16 0x00007f661b849720 in QMenu::popup(QPoint const&, QAction*) () from /usr/lib/libQt5Widgets.so.5
#17 0x00007f661b84bc2f in QMenu::exec(QPoint const&, QAction*) () from /usr/lib/libQt5Widgets.so.5

I also don't see the Find action in the toolbar when I start Dolphin. What's going on?

In D12094#244029, @sharvey wrote:

I don't believe there's any getting around having two entries in the available actions for toolbar/menu editing. Perhaps one could be given a different label and/or icon? It's awkward because they're both "find"-related actions, but have to be implemented separately.

I'd try to not add the second action to the actionCollection()

In D12094#246961, @elvisangelaccio wrote:

I'd try to not add the second action to the actionCollection()

Not adding the second action means we lose the ability to have the Find... button act as a toggle. I checked with David Faure, since he's the keeper of KXmlGui. He confirmed that two behaviors require two action-id entries. It seems we're up against a case of "that's just the way it works" and there isn't a way for me to cheat the system. I think we're stuck with a few limitations, at least until someone rewrites Dolphin in QML. (No, I'm not volunteering.)

While we all think having the toggling behaviour is a nice touch, duplicating entries in the shortcut dialog and in many other places is worse, IMO. Is there really no way to solve this? Can we override the shortcut handling somehow?

I searched and searched; read the API docs until my eyes ached. The first step (code-wise) in creating an action is adding an action-id to the ActionCollection. The next steps are adding a label, a shortcut key, and/or an icon.

A toggle is a separate type of object than a standard button or menu entry. Thus, it needs its own action-id.

I hesitated as long as I could before bothering David Faure. His email concludes with this subtle suggestion:

Two actions = two action names.

Make the statusText for the toggle action include the word toggle, to
differenciate them? The toolbar editor dialog shows the statusText near the
bottom.

I can patch that in if you like to see if it makes the duplicate entries more palatable and less confusing.

Otherwise, I'm sorry to say, there doesn't seem to be any way to get two for the price of one.

There should be a single action from the user's point of view:

• In the menu (with the shortcut for Find written besides it).
• In the toolbar (ideally displaying the toggle state).
• In the Configure Shortcuts dialog (clearly allowing to set the shortcut for Find).
• In the Configure Toolbars dialog.

If we cannot hide the second action from there, perhaps we can only have a single action, for which we can try to override functionality though (e.g. the shortcut should focus instead of toggle). I write "try to", because obviously I don't know how to solve this either. I guess nobody knows at this point, otherwise this would have been solved long ago. Eventually someone will find a way…

It's been two weeks of letting this problem stew, and a (partial) solution has just occurred to me.

Try the Split button. I don't believe it's a proper toggle button. The state doesn't change, only the icon.

(I don't have the code at hand; in the process of rebuilding my system.)

Perhaps we could change the magnifying glass icon to one with a red X over it to indicate "Find Off"? I think I could accomplish this by tracking a variable or two instead of replying on a toggle button and a second action-id.

It's theoretical for at least the rest of the day (grumble grumble technical difficulties), but I'm open to commentary.

Thanks for not forgetting about this…

In D12094#258177, @sharvey wrote:

Try the Split button. I don't believe it's a proper toggle button. The state doesn't change, only the icon.

Sure, you could look at this button to get inspiration from how it was implemented.

Perhaps we could change the magnifying glass icon to one with a red X over it to indicate "Find Off"?

There already is a standard behaviour for showing what we want (i.e. normal and toggled). Let's not work around our issues in implementing this by simply changing of how it should work. The Split button is special, because it isn't a toggle but can affect two different tabs.

Nevertheless, Find should work like Preview, only the shortcut handling should be different (focus instead of closing). Maybe you could try to attack the problem from the shortcut handling angle?

I never forgot; i just had to move it to the back of my mind and let it run as a background process. There's one big problem, though - which occurred to me while mowing my lawn. Changing the icon from to, say with a overlaid on it is that it's also going to change the icon in the drop-down menu as well. They're still linked, after all. And still no way around that.

I'll take another run at it once I get my machine fully operational and current again. Some Truly Bad Things happened while trying to get and keep Plasma current so I could work on something else. I think a cascading row of "Dr. Konqi" crash icons from KWin (which wouldn't stop short of a reboot) counts as something gone very wrong. Never a dull moment.