D22607: Fix compile warnings

fvogt created this revision. fvogt added reviewers: Plasma, broulik. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. fvogt requested review of this revision. REVISION SUMMARY Fix warnings printed during build. TEST PLAN Built it, no warnings anymore. REPOSITORY R85

D22607: Fix compile warnings

fvogt updated this revision to Diff 62176. fvogt added a comment. Do it differently. REPOSITORY R856 Plasma Browser Integration CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22607?vs=62175&id=62176 BRANCH master REVISION DETAIL https://phabricator.kde.org/D22607 AFFECTED F

D22607: Fix compile warnings

This revision was automatically updated to reflect the committed changes. Closed by commit R856:8a254b2ec2e0: Fix compile warnings (authored by fvogt). REPOSITORY R856 Plasma Browser Integration CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22607?vs=62176&id=62178 REVISION DETAIL

D21112: Support message response and reply callbacks

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. In D21112#498669 , @fvogt wrote: > Code looks good and seems to work fine in vivaldi, but I get an error in firefox: > > Promise resolved

D22623: Bind SQL parameters in firefox bookmarksrunner support

fvogt created this revision. fvogt added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. fvogt requested review of this revision. REVISION SUMMARY Previously (accidental) SQL injection was easily possible. TEST PLAN Could still list bookmarks. REP

D22641: Inject content script also into about:blank

fvogt added a comment. An iframe without source, the things web devs come up with... REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.kde.org/D22641 To: broulik, #plasma, fvogt Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenB

D22813: Don't give up if no results arrive after 500 ms, again.

fvogt requested changes to this revision. fvogt added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > runnerresultsmodel.cpp:56 > + > +m_hasMatches = false; > } Redundant REPOSITORY R112 Milou REVISION DETAIL https://phabricator.kde.or

D22623: Bind SQL parameters in firefox bookmarksrunner support

fvogt added a comment. I'll land in 24h if no objections. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22623 To: fvogt, #plasma Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreu

D22623: Bind SQL parameters in firefox bookmarksrunner support

This revision was automatically updated to reflect the committed changes. Closed by commit R120:7fdc614f4226: Bind SQL parameters in firefox bookmarksrunner support (authored by fvogt). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D22623?vs=62220&id=62803#toc REPOSITORY R120 Plasma Wo

D22596: [MPRIS Data Engine] Ignore non-standards compliant players

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. Code looks good, if you tested successfully with vlc this can IMO go in REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D22596 To: broulik, #plasma, fvogt Cc

D23078: Remove persistence from the kauth helper

fvogt created this revision. fvogt added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. fvogt requested review of this revision. REVISION SUMMARY The way the helper works allows everyone with access to elevate to root trivially, so persistence need

D23078: Remove persistence from the kauth helper

This revision was automatically updated to reflect the committed changes. Closed by commit R123:2ce7ccd2dff8: Remove persistence from the kauth helper (authored by fvogt). REPOSITORY R123 SDDM Configuration Panel (KCM) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D23078?vs=63511&id=

D23090: Handle media session callbacks even when the requested one isn't supported by the browser

fvogt added inline comments. INLINE COMMENTS > content-script.js:687 > +// still be able to handle it > +if (!["play", "pause", "seekbackward", "seekforward", > "previoustrack", "nexttrack", "skipad", "stop"].includes(name)) { > +th

D23091: Call media session callback with details

fvogt accepted this revision. fvogt added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > content-script.js:673 > +action: action > +// for seeking there's additional information one > would need to add > +

D23107: Slightly prettier debug

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. TIL that exists REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.kde.org/D23107 To: broulik, #plasma, davidedmundson, fvogt Cc: plasma-devel, LeGast00n, jra

D23100: Let settings work with arbitrary input controls

fvogt added inline comments. INLINE COMMENTS > options.js:37 > > function extensionCheckboxes() { > return document.querySelectorAll("#extensions-selection > input[type=checkbox][data-extension]"); Can be removed? > options.js:114 > + > +if (!settings[extension]) { > +

D23099: Allow sending a port message and receive a reply

fvogt added a comment. IMHO the member variable is really ugly. Messages with and without serial number have to be handled differently anyway, so why not introuce a new `handleMessage(event, json, serial)` method? INLINE COMMENTS > extension-utils.js:65 > +message.event = event; > +

D23100: Let settings work with arbitrary input controls

fvogt added inline comments. INLINE COMMENTS > broulik wrote in options.js:119 > What do you mean? `` -> `` and then just error out if there's no key defined. REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.kde.org/D23100 To: broulik, #plasma, fvogt Cc: pla

D23100: Let settings work with arbitrary input controls

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. Didn't test, but looks good REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.kde.org/D23100 To: broulik, #plasma, fvogt Cc: plasma-devel, LeGast00n, jraleig

D23099: Allow sending a port message and receive a reply

fvogt added inline comments. INLINE COMMENTS > broulik wrote in abstractbrowserplugin.h:56 > Why does it say "got hidden" when those two clearly have a different > signature? Because only the overridden overload is visible in the child classes. You can fix this by adding `using AbstractBrowser

D23099: Allow sending a port message and receive a reply

fvogt added inline comments. INLINE COMMENTS > extension.js:109 > > -if (!subsystem || !action) { > +if (replyToSerial < 0 && (!subsystem || !action)) { > return; Would need to be `"replyToSerial" in message` now, `undefined` is not smaller than 0 > pluginmanager

D21113: Allow hiding option items depending on available extension and version in the host

fvogt added inline comments. INLINE COMMENTS > options.js:207 > + > document.querySelectorAll("[data-requires-extension]").forEach((item) => { > +let requiredExtension = item.dataset.requiresExtension; > +let requiredMinimumVersion = > Number(item.data

D23151: Implement Web Share API through Purpose

fvogt requested changes to this revision. fvogt added a comment. This revision now requires changes to proceed. AFAICT this won't work on wayland and will also break the browser's native implementation once that actually exists. INLINE COMMENTS > content-script.js:22 > +// from > https://st

D23151: Implement Web Share API through Purpose

fvogt requested changes to this revision. fvogt added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > purposeplugin.cpp:122 > +urls.append(text); > +showShareMenu(shareJson, QStringLiteral("text/plain")); > +return {}; Doesn't

D23518: Disable media controls and tabsrunner in Firefox again

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. Accepting, but I REALLY don't like this as it makes the behaviour on firefox quite different compared to other browsers for no good reason. REPOSITORY R856 Plasma Browser Integration REVIS

D23568: Call window.postMessage with targetOrigin

fvogt added a comment. > Always provide a specific targetOrigin, not *, if you know where the other window's document should be located. Failing to provide a specific target discloses the data you send to any interested malicious site. I wonder whether that is relevant to us? REPOSITORY

D23568: Call window.postMessage with targetOrigin

fvogt added a comment. In D23568#522065 , @broulik wrote: > What we could also do which I just realized is that we could send a `CustomEvent` to the `window` instead of using `postMessage` > Basically what we did before except without a wrappe

D23568: Use custom event rather than window.postMessage

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. Tested, works fine REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.kde.org/D23568 To: broulik, #plasma, ognarb, fvogt, davidedmundson Cc: plasma-devel, LeG

D23686: Allow a setting to declaratively depend on another one

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. Works. I don't think the CSS indentation style works for `data-depends-settings-value="FALSE"` though, that would just be weird. REPOSITORY R856 Plasma Browser Integration REVISION DETAIL

D23709: RFC: Add option to store download origin url in xattr

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. LGTM, but maybe move into a separate method instead? There's already too much in `DownloadJob::update`... REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.k

D23714: Manipulate Document prototype for createElement

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. The same approach should probably be used for other overrides as well, like `navigator.mediaSession.setActionHandler` and `window.Audio`. That's unrelated to the bug though. REPOSITORY R85

D23773: Observe title tag only if there actually is a player

fvogt added inline comments. INLINE COMMENTS > content-script.js:333 > +if (!titleTagObserver) { > +oldPageTitle = ""; > + Why not document.title? REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.kde.org/D23773 To: broulik, #plasma, fvogt, ognarb

D23773: Observe title tag only if there actually is a player

fvogt added inline comments. INLINE COMMENTS > broulik wrote in content-script.js:333 > This is to store the name and only send a change when it is different from > last time I sent it. `document.title` will be the same as > `titleTag.innerText` rendering this check moot No, it won't be. Curre

D23773: Observe title tag only if there actually is a player

fvogt added inline comments. INLINE COMMENTS > broulik wrote in content-script.js:333 > An empty title is kinda pointless anyway, so this is on purpose It would still send an empty title on initialization or changes after the first change though. So if that's the purpose of the empty string ass

D23151: Implement Web Share API through Purpose

fvogt accepted this revision. fvogt added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > broulik wrote in purposeplugin.cpp:86 > `errorCode` and `errorMessage` are forwarded from the Purpose plugin, `error` > is our own. I could change `errorCode` to return a

D23117: [Look and feel] Add a way for LNF themes to manually specify a splash screen

fvogt added inline comments. INLINE COMMENTS > kcm.cpp:429 > //TODO: option to enable/disable apply? they don't seem required by UI > design > -setSplashScreen(m_selectedPlugin); > setLockScreen(m_selectedPlugin); If the if block above is not entered, this is not done anymore. Int

D24165: Add SettingsUtil utility class

fvogt added a comment. > Probably needs some Firefox ESR 60 testing to see whether it can deal with the class stuff... I can try if you'd like. INLINE COMMENTS > options.js:137 > }); > // When the extension is reloaded, any call to extension APIs throws > } catch (e)

D24165: Add SettingsUtil utility class

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. Appears to work on ESR REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.kde.org/D24165 To: broulik, #plasma, fvogt, ognarb Cc: plasma-devel, LeGast00n, The-

D24191: Let plugin add additional status information

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. Seems a bit weird to merge it at the top-level instead of adding a new key, but it's unlikely to result in abuse. REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabr

D24191: Let plugin add additional status information

fvogt added inline comments. INLINE COMMENTS > broulik wrote in abstractbrowserplugin.h:47 > Not too happy about making it public, but it is used by `Settings`. Other > alternative would be to make a getter in `PluginManager` which then calls > this... or also making `Settings` a `friend class`

D24225: [Notifications] Add spacing after positioning popup for bottom panel

fvogt added a comment. Seems to work here REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D24225 To: broulik, #plasma, #vdg Cc: fvogt, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai,

D24194: Add per-domain media controls blacklist

fvogt added a comment. AFAICT it doesn't reload the mpris state in the content-script immediately when settings change, can that be implemented? IMO the blacklist should be more than the domain, it should test the same conditions as CORS, so protocol, domain and port. INLINE COMMENTS >

D24203: Add settings change listener

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. Seems to work on FF ESR REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.kde.org/D24203 To: broulik, #plasma, fvogt, ognarb Cc: plasma-devel, LeGast00n, The

D9597: Move calls depending on $DISPLAY from startplasmacompositor to startplasma

This revision was automatically updated to reflect the committed changes. Closed by commit R120:32932b44e6f0: Move calls depending on $DISPLAY from startplasmacompositor to startplasma (authored by fvogt). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org

D9689: [WIP] Add a per-process CPU usage graph shown in the process list

fvogt created this revision. fvogt added a reviewer: Plasma. Restricted Application added a project: Plasma. fvogt requested review of this revision. REVISION SUMMARY By adding a new PercentageHistoryRole returning a QVector with timestamps and values, the delegate can show the history of the

D9689: [WIP] Add a per-process CPU usage graph shown in the process list

fvogt updated this revision to Diff 24804. fvogt added a comment. Remove the history tail before appending the head to never exceed the reserved vector size REPOSITORY R111 KSysguard Library CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9689?vs=24803&id=24804 BRANCH history

D9689: [WIP] Add a per-process CPU usage graph shown in the process list

fvogt updated this revision to Diff 24836. fvogt added a comment. Instead of updating the last entry's timestamp if the value is the same, add a new entry only if the latest entry has a certain age. Otherwise, the interpolation results would look differently and processes had different

D9689: [WIP] Add a per-process CPU usage graph shown in the process list

fvogt updated this revision to Diff 24838. fvogt added a comment. Forgot to save ProcessModel.cpp before git commit. REPOSITORY R111 KSysguard Library CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9689?vs=24836&id=24838 BRANCH history REVISION DETAIL https://phabricator.kde

D9689: [WIP] Add a per-process CPU usage graph shown in the process list

fvogt updated this revision to Diff 24921. fvogt added a comment. Address review comments. Set a unified alpha for percentage marker and history graph to avoid sudden changes: 33%: Percentage marker 66%: History graph 100%: Text Screenshot is linked in the test plan. REPOSITOR

D9689: Add a per-process CPU usage graph shown in the process list

fvogt retitled this revision from "[WIP] Add a per-process CPU usage graph shown in the process list" to "Add a per-process CPU usage graph shown in the process list". fvogt edited the test plan for this revision. REPOSITORY R111 KSysguard Library REVISION DETAIL https://phabricator.kde.org

D9689: Add a per-process CPU usage graph shown in the process list

fvogt updated this revision to Diff 24935. fvogt added a comment. Rebase on current master. (Apparently my local master wasn't setup to follow origin...) REPOSITORY R111 KSysguard Library CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9689?vs=24921&id=24935 BRANCH history REV

D9738: Support font/ttf and font/otf mimetypes in kfontinst

fvogt created this revision. fvogt added a reviewer: Plasma. Restricted Application added a project: Plasma. fvogt requested review of this revision. REVISION SUMMARY Those mimetypes are new and kfontinst/kfontview/fontthumbnail weren't aware of that. As the old types are aliases of the new

D9738: Support font/ttf and font/otf mimetypes in kfontinst

fvogt added inline comments. INLINE COMMENTS > anthonyfieroni wrote in Misc.cpp:293 > So why not > > return CFontList::fontMimeTypes.contains(mime); > > In other places too. It's probably not linked together. Anyway, this is just supposed to be a simple fix and not a refactor, stuff like t

D9738: Support font/ttf and font/otf mimetypes in kfontinst

This revision was automatically updated to reflect the committed changes. Closed by commit R119:c9e036e24873: Support font/ttf and font/otf mimetypes in kfontinst (authored by fvogt). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9738?vs=24955&id=25062

D9689: Add a per-process CPU usage graph shown in the process list

fvogt updated this revision to Diff 25109. fvogt added a comment. Address review comments. REPOSITORY R111 KSysguard Library CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9689?vs=24935&id=25109 BRANCH history REVISION DETAIL https://phabricator.kde.org/D9689 AFFECTED FILES

D9072: make ksshaskpass work with git-lfs

fvogt added a comment. Looks good to me, but I wonder whether > KWallet writes an error to stderr when a 0-winId is given to openWallet() which is received by git-lfs and aborts the operation. should be fixed more in a more general way as well. REPOSITORY R105 KDE SSH Password Dia

D9495: Properly detect Gallium drivers with newer Mesa

This revision was automatically updated to reflect the committed changes. Closed by commit R108:e302f87598de: Properly detect Gallium drivers with newer Mesa (authored by fvogt). Restricted Application edited projects, added Plasma; removed KWin. REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE

D9689: Add a per-process CPU usage graph shown in the process list

fvogt updated this revision to Diff 25206. fvogt added a comment. Declare PercentageHistoryEntry as Q_PRIMITIVE_TYPE REPOSITORY R111 KSysguard Library CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9689?vs=25109&id=25206 BRANCH history REVISION DETAIL https://phabricator.kde

D9845: Fix source actions in the configuration

fvogt created this revision. fvogt added reviewers: Plasma, apol. Restricted Application added a project: Plasma. fvogt requested review of this revision. REVISION SUMMARY It showed "" instead. TEST PLAN Shows source actions now. REPOSITORY R134 Discover Software Store BRANCH master RE

D9845: Fix source actions in the configuration

This revision was automatically updated to reflect the committed changes. Closed by commit R134:86ab5059134a: Fix source actions in the configuration (authored by fvogt). REPOSITORY R134 Discover Software Store CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9845?vs=25245&id=25268 RE

D9873: Manually take XKB_DEFAULT_{RULES,MODEL,LAYOUT,VARIANT,OPTIONS} into account

fvogt created this revision. fvogt added reviewers: Plasma, graesslin. Restricted Application added a project: KWin. Restricted Application added subscribers: KWin, kwin. fvogt requested review of this revision. Restricted Application edited projects, added Plasma; removed KWin. REVISION SUMMARY

D9873: Manually take XKB_DEFAULT_{RULES,MODEL,LAYOUT,VARIANT,OPTIONS} into account

fvogt added a comment. Restricted Application edited projects, added Plasma; removed KWin. In https://phabricator.kde.org/D9873#190789, @graesslin wrote: > I wasn't aware of this secure_getenv functionality. Me neither... > Is that also in place after the process has completel

D9873: Manually take XKB_DEFAULT_{RULES,MODEL,LAYOUT,VARIANT,OPTIONS} into account

fvogt updated this revision to Diff 25351. fvogt added a comment. Restricted Application edited projects, added KWin; removed Plasma. Move the added functions from the class into the xkb.cpp's static scope. REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9873?v

D9873: Manually take XKB_DEFAULT_{RULES,MODEL,LAYOUT,VARIANT,OPTIONS} into account

fvogt closed this revision. fvogt added a comment. Restricted Application edited projects, added KWin; removed Plasma. Fixed up the trailing whitespace in line 165 and landed: https://commits.kde.org/kwin/eb69e87288d37fdb13eca32ca807ed8279f912af REPOSITORY R108 KWin REVISION DETAIL https

D9689: Add a per-process CPU usage graph shown in the process list

This revision was automatically updated to reflect the committed changes. Closed by commit R111:f28072080b89: Add a per-process CPU usage graph shown in the process list (authored by fvogt). REPOSITORY R111 KSysguard Library CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9689?vs=2520

D9072: make ksshaskpass work with git-lfs

fvogt added a comment. In https://phabricator.kde.org/D9072#189550, @mkoller wrote: > In https://phabricator.kde.org/D9072#189303, @fvogt wrote: > > > Looks good to me, but I wonder whether > > > > > KWallet writes an error to stderr when a 0-winId is given to openWallet() which i

D9998: Actually quit threads nicely

fvogt created this revision. fvogt added reviewers: Plasma, ivan. Restricted Application added a project: Plasma. fvogt requested review of this revision. REVISION SUMMARY https://phabricator.kde.org/R161:27c0245b1715044cf4d401f1c9d7e7a915a4f3c5 ("[resources] Nicely quit threads") has no effe

D9998: Actually quit threads nicely

fvogt added a reviewer: anthonyfieroni. REPOSITORY R161 KActivity Manager Service REVISION DETAIL https://phabricator.kde.org/D9998 To: fvogt, #plasma, ivan, anthonyfieroni Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9998: Actually quit threads nicely

fvogt added a comment. In https://phabricator.kde.org/D9998#193836, @anthonyfieroni wrote: > If it's not running event loop why run is called? That's what a QThread is: > A QThread object manages one thread of control within the program. QThreads begin executing in run(). By

D9072: make ksshaskpass work with git-lfs

fvogt accepted this revision. fvogt added a comment. This revision is now accepted and ready to land. In https://phabricator.kde.org/D9072#193866, @mkoller wrote: > so you mean instead of a one line change fiddling with file descriptors is a better way ? > I don't think so. > > And

D9998: Actually quit threads nicely

fvogt added inline comments. INLINE COMMENTS > lbeltrame wrote in Resources.cpp:60 > What's the reason for 1500 here? It's the `// initial delay before processing the events` plus some time for it to process the last request. I can add a comment if you want to. REPOSITORY R161 KActivity Mana

D9998: Actually quit threads nicely

fvogt updated this revision to Diff 25709. fvogt added a comment. Add comments to explain the magic values REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9998?vs=25694&id=25709 BRANCH Plasma/5.12 REVISION DETAIL https://phabricator.k

D9998: Actually quit threads nicely

This revision was automatically updated to reflect the committed changes. Closed by commit R161:ed092b96f7af: Actually quit threads nicely (authored by fvogt). REPOSITORY R161 KActivity Manager Service CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9998?vs=25709&id=25737 REVISION DE

D9998: Actually quit threads nicely

fvogt added inline comments. INLINE COMMENTS > ivan wrote in ResourceScoreMaintainer.cpp:62 > If that is enough, then why not just `wait()`? Simple: `wait()` waits for an infinite amount of time if `run()` never returns. If that happens (`run()` is stuck), we prefer a crash over silently wastin

D10188: Sanitise notification HTML

fvogt requested changes to this revision. fvogt added a comment. This revision now requires changes to proceed. This is required for 5.8 as well. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10188 To: davidedmundson, #plasma, fvogt Cc: fvogt, plasma-devel

D10188: Sanitise notification HTML

fvogt added inline comments. INLINE COMMENTS > notificationsanitizer.cpp:34 > +t.replace(QLatin1String("\n"), QStringLiteral("")); > +// Now remove all inner whitespace (\ns are already s > +t = t.simplified(); Missing `)` > notificationsanitizer.cpp:57 > +const QString

D10188: Sanitise notification HTML

fvogt added a comment. Looks good to me. I didn't try it out myself though, but the unit tests look almost complete to me: Can you add a unit test for obviously broken HTML, like `https://phabricator.kde.org/D10188 To: davidedmundson, #plasma, fvogt Cc: fvogt, plasma-devel, ZrenBot, prog

D10188: Sanitise notification HTML

fvogt accepted this revision. This revision is now accepted and ready to land. REPOSITORY R120 Plasma Workspace BRANCH master REVISION DETAIL https://phabricator.kde.org/D10188 To: davidedmundson, #plasma, fvogt Cc: aacid, fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D10234: LibInput: Queue native libinput events instead of LibInput::Event

fvogt created this revision. fvogt added reviewers: Plasma, graesslin. Restricted Application added a project: KWin. Restricted Application added subscribers: KWin, kwin. fvogt requested review of this revision. Restricted Application edited projects, added Plasma; removed KWin. REVISION SUMMARY

D10234: LibInput: Queue native libinput events instead of LibInput::Event

fvogt edited the summary of this revision. Restricted Application edited projects, added KWin; removed Plasma. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D10234 To: fvogt, #plasma, graesslin Cc: kwin, plasma-devel, #kwin, iodelay, bwowk, ZrenBot, progwolff, lesliezhai,

D10234: LibInput: Queue native libinput events instead of LibInput::Event

fvogt added a comment. Restricted Application edited projects, added KWin; removed Plasma. In https://phabricator.kde.org/D10234#199148, @graesslin wrote: > I had another idea: when the Event gets processed and the m_device is null, we just update it again from the event. > > Somethin

D10236: [libinput] Ensure Event::device returns a proper Device

fvogt added a comment. Restricted Application edited projects, added KWin; removed Plasma. Now you can remove the initial setting of `m_device` as well. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D10236 To: graesslin, #kwin, #plasma, fvogt Cc: plasma-devel, kwin, io

D10236: [libinput] Ensure Event::device returns a proper Device

fvogt added a comment. Restricted Application edited projects, added KWin; removed Plasma. In https://phabricator.kde.org/D10236#199259, @graesslin wrote: > In https://phabricator.kde.org/D10236#199252, @fvogt wrote: > > > Now you can remove the initial setting of `m_device` as well.

D10236: [libinput] Ensure Event::device returns a proper Device

fvogt accepted this revision. fvogt added a comment. Restricted Application edited projects, added KWin; removed Plasma. Tested, works! REPOSITORY R108 KWin BRANCH libinput-event-device REVISION DETAIL https://phabricator.kde.org/D10236 To: graesslin, #kwin, #plasma, fvogt, avaragic C

D10236: [libinput] Ensure Event::device returns a proper Device

fvogt added a comment. Restricted Application edited projects, added Plasma; removed KWin. I'm actually in favor of a tar respin for 5.12.0 in this case. What do you think? REPOSITORY R108 KWin BRANCH libinput-event-device REVISION DETAIL https://phabricator.kde.org/D10236 To: graess

D10151: Correction with the & problem in tabs

fvogt reopened this revision. fvogt added a comment. This revision is now accepted and ready to land. This needs to be commited to Plasma/5.12 as well, probably also Plasma/5.8. REPOSITORY R106 KSysguard REVISION DETAIL https://phabricator.kde.org/D10151 To: carlavilla, mlaurent, ngraham

D10315: [Notifications] Fix grouping

fvogt added inline comments. INLINE COMMENTS > notificationsengine.cpp:266 > const int WORD_PER_MINUTE = 250; > -int count = summary.length() + body.length(); > +int count = summary.length() + (body.length() - 33); // 33 is the length > of and tags > I don't like magic numbers,

D10315: [Notifications] Fix grouping

fvogt added inline comments. INLINE COMMENTS > notificationsengine.cpp:230 > +if (previousBody != bodyFinal) { > +bodyFinal = previousBody + QStringLiteral("") + > bodyFinal; > } This results in `...TextSome text...` AFAICT. This also means that the if

D10315: [Notifications] Fix grouping

fvogt added inline comments. INLINE COMMENTS > fvogt wrote in notificationsengine.cpp:230 > This results in `...TextSome text...` AFAICT. > > This also means that the if condition is always false. According to @broulik the first part is expected, which IMO needs a comment as it does very much

D10404: Close the plasmoid after updates got installed

fvogt created this revision. fvogt added reviewers: lukas, jgrulich. fvogt requested review of this revision. REVISION SUMMARY The plasmoid searches for the next batch of updates which might be misleading. Related: https://bugs.kde.org/show_bug.cgi?id=390143 TEST PLAN Installed updates,

D10425: Fix appearance of the logout dialog on wayland

fvogt created this revision. fvogt added reviewers: Plasma, graesslin, davidedmundson. Restricted Application added a project: Plasma. fvogt requested review of this revision. REVISION SUMMARY The logout dialog has a taskbar entry and appears like a regular window on wayland. By setting its ro

D10425: Fix appearance of the logout dialog on wayland

fvogt updated this revision to Diff 26874. fvogt added a comment. Don't use the setRole(Notification) hack. That causes it to be drawn below fullscreen windows. OSD might work, but those don't normally accept input. So for now just use setSkipTaskbar(true); REPOSITORY R120 Plasma Worksp

D10425: Improve appearance of the logout dialog on wayland

fvogt retitled this revision from "Fix appearance of the logout dialog on wayland" to "Improve appearance of the logout dialog on wayland". fvogt edited the summary of this revision. fvogt edited the test plan for this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabri

D10425: Improve appearance of the logout dialog on wayland

fvogt added a comment. > OSD might work, but those don't normally accept input. "normally" = "it shouldn't, but when testing it it does accept input just fine" @graesslin is it a bug that `setRole(OnScreenDisplay);` makes it work exactly like on X11 on wayland? i.e. is it intentional

D10425: Improve appearance of the logout dialog on wayland

fvogt added a comment. In https://phabricator.kde.org/D10425#203972, @graesslin wrote: > I'm against this change. The window is set to fullscreen, but that doesn't work due to a Qt bug. Is the referenced patch enough to make it work? AFAICT it would also need a change in kwayland

D10429: Disable the title bar separator by default

fvogt created this revision. fvogt added reviewers: VDG, Plasma, ngraham. Restricted Application added a project: Plasma. fvogt requested review of this revision. REVISION SUMMARY Since the removal of the heuristics this seems to be very much disliked by many. As there is now a configuration

D10425: Improve appearance of the logout dialog on wayland

fvogt updated this revision to Diff 26891. fvogt added a comment. Use a different workaround: wl-shell instead of xdg-shell. REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10425?vs=26874&id=26891 BRANCH Plasma/5.12 REVISION DETAIL https://pha

D10425: Improve appearance of the logout dialog on wayland

fvogt edited the summary of this revision. fvogt edited the test plan for this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D10425 To: fvogt, #plasma, graesslin, davidedmundson, bshah Cc: ngraham, bshah, plasma-devel, ZrenBot, progwolff, lesliezhai,

D10425: Improve appearance of the logout dialog on wayland

This revision was automatically updated to reflect the committed changes. Closed by commit R120:4d7c1345b784: Improve appearance of the logout dialog on wayland (authored by fvogt). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10425?vs=26891&id=2692

D10668: Fix PowerDevil shortcuts migration

fvogt added inline comments. INLINE COMMENTS > davidedmundson wrote in powerdevilapp.cpp:177 > You'd think so, but no. > > setShortcut(QAction, sequence, KGlobalAccel::NoAutoloading) > would do exactly that > > setShortcut(QAction, sequence, KGlobalAccel::Autoloading) > (which this defaul

<    1   2   3   4   5   6   7   >