Yes (although I would then swap document to be after text/, to have all the "groups" together, and then the "substring searches").
In any case feel free to push.

Yes, but then startsWith() would be even more correct (and slightly faster) than contains().

I agree, this should go in.

One can also put %{appname} in his $QT_MESSAGE_PATTERN for this :-)

Qt (at shortcut recording time) sees this as Meta+Shift+Print while KWindowSystem (xcbKeyPressEventToQt) sees it as Meta+Shift+SysReq.

Alt+Shift+right bug fixed in https://phabricator.kde.org/D7829

Thanks for the reports. These shortcuts get converted correctly (https://commits.kde.org/kwindowsystem/af6d5ab12b2d99b39a73ed8df903368470ded55b), so the problem must be in the shift handling in xcbKeyPressEventToQt().

Add both flag values, so that the current patches still work and don't need to be changed again if we toggle the default.

If all you need is <APPDIR>, you can use QCoreApplication::applicationDirPath().

In D7580#144518, @kossebau wrote:

When I read this initially, I guessed this method is just about the view state. But is also bound to data-pulling by the kpart, given that the default implementation explicitely calls openUrl() with the url stored in the datastream. Which might make sense for simple-to-use API with the non-stream use cases. But leaves out the stream-based data-pushing usage.

I'll take silence as approval, this should be non-controversial...

The CMake buildsystem is done.

In D7580#142971, @kossebau wrote:

"how would zoom and other custom state properties be save and retrieved again" -> using BrowserExtension's saveState/restoreState as usual, no? I'm not 100% sure about the interaction with streaming, but normally that happens after opening the url anyway, so it should be unrelated.

Oh, somehow missed those methods. Possibly was blinded by KParts::OpenUrlArguments::xOffset()/yOffset() and since then assumed that state restoring was supposed to be done only via those arguments.

Hmm. Emmanuel might have a point.
Looking at KIO jobs usage, I see a lot that are user-triggered and would benefit from privilege-execution support.
All of KIO's filewidgets library (filedialog etc.) obviously qualifies. KTextEditor for editing root-owned files, KParts for vieweing them, and then the same in most apps that use KIO to open and/or save files.

separate commits are easier to review, so that's fine

Has this been reported upstream, so the cdda people can fix it properly?

Oops, yes, you're completely right. I got distracted by the word close, but it's a different meaning in closeStream and closeUrl. Apologies for the confusion.

Thanks for the fix!

It's opt-in because it should only happen for user-triggered operations.
An application copying some config file in the background (or any other internal file operation) shouldn't bother the user with prompts.

Maybe it would be safer/simpler to use a different QObject for watching the menus.

(strange that Phabricator still says "Needs Review" when this has two approvals)

Also please improve the commit log to be more self contained (unit tests for what? And "T6561" will mean nothing to someone doing git log).

Looks good, just some minor things.

Please use arc diff to upload patches so that context is available.

Patch for more docu: https://phabricator.kde.org/D7612

Well, *if*. Let's first find out if this is the case. I'm sort for time right now, feel free to investigate if you have time.

Ah! I know how to improve the readability of (auto ret = execWithElevatedPrivilege)... please rename all "ret" variables to "err" (short for error, which is already taken), or if you prefer something longer, maybe execError ? eWEPError ?

RemoveAutority contains RemoveUserInfo | RemovePort, so you probably don't want that either.

Is there a reason why Konqueror is added in CC of all dolphin merge requests? Is that done manually or automatically? I don't see the relevance for konqueror, especially for a patch for DolphinMainWindow (no effect on dolphinpart).

Ah, phabricator is strange, I have indeed modified the commit log before doing arc diff but it didn't update it here.

Provide servicetype file to the cmake macro so that the priority is stored as an int.

In D7533#139802, @bshah wrote:

One thing I don't understand is the need for toString().toInt(),
or in other words, why my json file with a numeric value gets turned
into a string value when using qtplugininfo to inspect the plugin:

Maybe you need to add following in the src/widgets/kurifilterplugin.desktop

[PropertyDef::X-KDE-InitialPreference]
Type=int

It will be tested at every "make test" in kio and kpackage ;-)

ping?

• Fix leak introduced by first version of the commit, we need to make a copy of the idle list before clearing it.