Bug 196035 - middle clicking on archive files in dolphin does not open them in a new tab
Review Request #110487 - Created May 17, 2013 and submitted
When 'browse through archives' is enabled, open archive files like folders on middle clicking, context menu -> new tab action and context menu -> new window action.
Middle clicking or context menu -> new tab action opens the archive in a new tab, if 'browse through archives' is active. Context menu -> new window action opens the archive in a new window, if 'browse through archives' is active. Works for me.
Thanks for the patch! The approach looks good, but I think that a function DolphinView::itemCanBeOpenedAsFolder(const KFileItem& item, KUrl& url) which has side effects on the argument 'url' is not a perfect solution. I see though that there is no obvious more elegant alternative :-( I'll think about this... Maybe we could make DolphinView::itemCanBeOpenedAsFolder(KFileItem) only check if opening the item is possible and adjust the 'protocol' of the URL somewhere else (if necessary), like in DolphinView's constructor and setUrl()? One could then also move the 'GeneralSettings::browseThroughArchives()' check out of that function - I see no reason why middle-clicking an archive should not open it even if that setting is disabled. What do you think?
Review request changed
Replaced "bool DolphinView::itemCanBeOpenedAsFolder(const KFileItem& item, KUrl& url)" by "KUrl DolphinView::openItemAsFolderUrl(const KFileItem& item)", which returns an empty url if the item can't be opened as a folder but a valid and adjusted url if the item can be opened as a folder. Also adjusted the code, so that multiple selected archives can be opened in multiple tabs (like open multiple folders in multiple tabs - commit 05d9210e)
Revision 2 (+83 -40)
I haven't tested it, but this looks to me like we never arrive here when the protocol is "search:". Have you tested this?
If my assumption above is correct, I would revert all changes above this 'else' and replace your 'else' by else if (!DolphinView::openItemAsFolderUrl(item).isEmpty())
I'm still not sure if this is the right place for the "browse through archives" settings check. IMHO, this setting refers to "browse through archives when clicking them, rather than opening Ark". When an archive is middle-clicked (or "Open in new Tab" is selected in the context menu), the user does obviously not want to open the archive in Ark. So I think that we should only check this setting in DolphinViewContainer::slotItemActivated(KFileItem) But maybe that's a matter of taste? Or is there a good reason why one should never open an archive as a folder if that setting is disabled (unless the user manually enters the URL of the archive in the location bar)? What do other people think about this?
@Frank: If you wonder why I have added the "bool browseThroughArchives" with the default value "true" - We need this, because we have to call this function anyway in "DolphinViewContainer::slotItemActivated" to handle the Desktop files properly. But the only place we take care of the browse through archives settings is in "DolphinViewContainer::slotItemActivated", all other places allow browse through archives by default.
Thanks, looks good from my point of view! I like your "bool browseThroughArchives" idea, probably that's the easiest solution for the issue. Thanks for your work! I think that many people will appreciate this improvement.