fvogt (Fabian Vogt)
User

Projects

User does not belong to any projects.

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Tuesday

  • Clear sailing ahead.

User Details

User Since
May 24 2016, 11:24 AM (159 w, 5 d)
Availability
Available

Recent Activity

Fri, Jun 14

fvogt updated the diff for D21605: Don't give up if no results arrive after 500ms.

Remove whitespace change. Will land on Monday if no objections.

Fri, Jun 14, 12:24 PM · Plasma

Thu, Jun 13

fvogt accepted D21785: Don't track subsystem status.

Indeed unused, but I wonder whether doing it explicitly in D21113 is prettier...

Thu, Jun 13, 8:56 PM · Plasma

Tue, Jun 11

fvogt added a comment to D8532: [WIP] Restrict file extractor with Seccomp.
In D8532#478224, @bruns wrote:

I totally agree with fvogt here - the extractors should just receive a readonly file descriptor.

For this, there are several steps required:

  1. let the extractors work with file descriptors (KFileMetaData)
  2. make sure the extractor plugins are fully initialized before receiving file descriptors
  3. actually feed file descriptors to the extractor

    (1.) is trivial for some extractors (e.g. taglib), for others it may be hard. (2.) depends on several things - the plugins must be instantiated early (which clashes with the lazy loading), and the plugin may not load any external resources later on.

    Using file descriptors has another benefit - currently, the file is stat'ed and so on, and then the corresponding path is fed to the extractor. It would be much better to open the file, use fstatat and friends, run the extractor and close the file again.
Tue, Jun 11, 5:04 PM · Baloo, Frameworks
fvogt added a comment to D21748: Replace the excludeRange mode setting when already available.

Untested, but looks good. Seems to apply to Plasma/5.12 as well.

Tue, Jun 11, 4:49 PM · Plasma

Mon, Jun 10

fvogt requested changes to D21725: Use a native application for starting plasma.
Mon, Jun 10, 5:28 PM · Plasma
fvogt committed R112:a1b94d503443: Merge branch 'Plasma/5.16' (authored by fvogt).
Merge branch 'Plasma/5.16'
Mon, Jun 10, 9:17 AM
fvogt committed R112:9d5921fb21c7: Enforce use of the reset timer for clearing the model (authored by fvogt).
Enforce use of the reset timer for clearing the model
Mon, Jun 10, 9:15 AM
fvogt committed R112:d7ef561dc8c7: Merge branch 'Plasma/5.12' into Plasma/5.16 (authored by fvogt).
Merge branch 'Plasma/5.12' into Plasma/5.16
Mon, Jun 10, 9:15 AM
fvogt closed D21670: Enforce use of the reset timer for clearing the model.
Mon, Jun 10, 9:15 AM · Plasma

Sat, Jun 8

fvogt requested review of D21670: Enforce use of the reset timer for clearing the model.
Sat, Jun 8, 5:00 PM · Plasma

Fri, Jun 7

fvogt committed R119:1c65dc7f7aa6: Merge branch 'Plasma/5.16' (authored by fvogt).
Merge branch 'Plasma/5.16'
Fri, Jun 7, 9:24 AM
fvogt committed R119:df81b30e11a6: kcm_fonts: Fix tracking of configuration changes (authored by fvogt).
kcm_fonts: Fix tracking of configuration changes
Fri, Jun 7, 9:12 AM
fvogt closed D21640: kcm_fonts: Fix tracking of configuration changes.
Fri, Jun 7, 9:12 AM · Plasma
fvogt committed R119:cf3ac466a187: kcm_fonts: Initialize variables properly (authored by fvogt).
kcm_fonts: Initialize variables properly
Fri, Jun 7, 9:10 AM
fvogt closed D21641: kcm_fonts: Initialize variables properly.
Fri, Jun 7, 9:10 AM · Plasma
fvogt requested review of D21641: kcm_fonts: Initialize variables properly.
Fri, Jun 7, 9:04 AM · Plasma
fvogt requested review of D21640: kcm_fonts: Fix tracking of configuration changes.
Fri, Jun 7, 9:00 AM · Plasma

Thu, Jun 6

fvogt accepted D21554: Allow setting fullscreen via MPRIS.
Thu, Jun 6, 5:15 PM · Plasma
fvogt updated the test plan for D21554: Allow setting fullscreen via MPRIS.
Thu, Jun 6, 5:09 PM · Plasma
fvogt updated the diff for D21607: Don't delay emission of matchesChanged indefinitely.

New algorithm with no delay if not necessary.

Thu, Jun 6, 10:07 AM · Frameworks
fvogt added a comment to D21607: Don't delay emission of matchesChanged indefinitely.

So for the stable branches I'd like to keep the current version of the diff with a latency of [100,599] while for master something like the above with a latency of [0,500] can be tried.

Thu, Jun 6, 10:06 AM · Frameworks

Wed, Jun 5

fvogt added a comment to D21607: Don't delay emission of matchesChanged indefinitely.

I'm thinking about doing it completely differently now though, with a 0 latency case (untested):

if(lastMatchChangeSignalled.hasExpired(250)) {
    matchChangeTimer.stop();
    emit q->matchesChanged(context.matches());
} else {
    matchChangeTimer.start(250 - lastMatchChangeSignalled.elapsed());
}
Wed, Jun 5, 4:45 PM · Frameworks
fvogt added a comment to D21607: Don't delay emission of matchesChanged indefinitely.

This would emit the signal more often, but wouldn't

if (!matchChangeTimer.isActive())
  matchChangeTimer.start(100)

achieve essentially the same?

Wed, Jun 5, 4:34 PM · Frameworks
fvogt added inline comments to D21605: Don't give up if no results arrive after 500ms.
Wed, Jun 5, 3:32 PM · Plasma
fvogt updated the diff for D21605: Don't give up if no results arrive after 500ms.

Code duplication is better than polluting the API.

Wed, Jun 5, 3:31 PM · Plasma
fvogt requested review of D21607: Don't delay emission of matchesChanged indefinitely.
Wed, Jun 5, 3:23 PM · Frameworks
fvogt requested review of D21606: RFC: ThreadWeaver Job Decorators not used properly and have no effect.
Wed, Jun 5, 3:09 PM · Frameworks
fvogt requested review of D21605: Don't give up if no results arrive after 500ms.
Wed, Jun 5, 2:59 PM · Plasma
fvogt accepted D21594: Allow media controls and tabs runner in incognito mode for Firefox 67.
Wed, Jun 5, 9:15 AM · Plasma

Wed, May 29

fvogt accepted D21478: Fix passing local file paths on the command line.
Wed, May 29, 1:18 PM
fvogt added inline comments to D21478: Fix passing local file paths on the command line.
Wed, May 29, 1:01 PM
fvogt accepted D21476: [Downloads Plugin] Update existing job when being signalled creation of existing job.
Wed, May 29, 9:53 AM · Plasma

Tue, May 28

fvogt added inline comments to D16648: Open externally called files/directories in new tabs.
Tue, May 28, 7:09 AM · Dolphin

Mon, May 27

fvogt added a comment to D21038: Also store the player's frame ID.

Couldn't find anything wrong, but I'm currently missing the overview of all MPRIS stuff in here to actually review properly...

Mon, May 27, 12:35 PM · Plasma
fvogt requested changes to D21112: Support message response and reply callbacks.

browser.runtime.sendMessage does not seem to have a callback parameter.

Mon, May 27, 12:00 PM · Plasma
fvogt added inline comments to D21113: Allow hiding option items depending on available extension and version in the host.
Mon, May 27, 11:55 AM · Plasma

Wed, May 22

fvogt added a comment to D16648: Open externally called files/directories in new tabs.

IMO some of the slots taking a QString/QStringList instead of their QUrl counterpart should have a comment why this is the case.
From a first glance a method isUrlOpen(QString) looks weird, but with a comment that would be come clearer.

Wed, May 22, 7:02 AM · Dolphin

Tue, May 21

fvogt added a comment to D16648: Open externally called files/directories in new tabs.

It looks like several QUrls were changed to QString here, why? Most of those QStrings, if not all, actually contain URLs.

If I remember correctly Qt methods that are publicised on DBus cannot have parameters as QList<QUrl>, hence I had to use QStringList. This seems to happen often enough that Qt even has helper functions to do the conversion. There are some more conversions to avoid unnecessarily changing method signatures of existing functions.

Not quite sure whether Qt supports that, but you could overload the methods of MainWindow so that the slots accept a QStringList and just pass it on.

QUrl::fromStringList and QUrl::toStringList were created for the express purpose of dealing with DBus calls: https://gitlab.com/pteam/pteam-qtbase/commit/6b9545a980f09d8fb034d930cfe67f6110357d0c.
In fact @dfaure was the one who created these functions himself :)
I think I'll stick with using those static methods unless there is a different reason to objection.

Tue, May 21, 5:57 PM · Dolphin
fvogt added a comment to D16648: Open externally called files/directories in new tabs.

It looks like several QUrls were changed to QString here, why? Most of those QStrings, if not all, actually contain URLs.

If I remember correctly Qt methods that are publicised on DBus cannot have parameters as QList<QUrl>, hence I had to use QStringList. This seems to happen often enough that Qt even has helper functions to do the conversion. There are some more conversions to avoid unnecessarily changing method signatures of existing functions.

Tue, May 21, 5:35 PM · Dolphin
fvogt added a comment to D21274: Don't assign a QtObject to a model.

Yes, but the workaround here has no actual downsides, and the upside is avoiding a guaranteed crash with the still latest non-development release of Qt.

Tue, May 21, 12:34 PM · Plasma
fvogt added a comment to D16648: Open externally called files/directories in new tabs.

It looks like several QUrls were changed to QString here, why? Most of those QStrings, if not all, actually contain URLs.

Tue, May 21, 12:17 PM · Dolphin
fvogt accepted D21274: Don't assign a QtObject to a model.

Tested on top of the 5.16 beta, works fine. Please add a comment referencing the QTBUG though, so that this doesn't get reintroduced by accident.

Tue, May 21, 11:54 AM · Plasma

May 16 2019

fvogt added inline comments to D21228: [Touchpad KCM] Load previous setting on reboot.
May 16 2019, 8:51 AM · Plasma

May 15 2019

fvogt added a comment to D20186: [libinput-touchpad-kcm] Use wayland specific touchpad KCM UI when libinput is used on X11.

Also keep in mind that it's possible to have global config files in /etc/X11/xorg.conf.d/ I have one such file there myself that I made to work around the lack of this feature being implemented yet:

$  cat /etc/X11/xorg.conf.d/99-libinput.conf
Section "InputClass"
        Identifier "libinput touchpad catchall"
        MatchIsTouchpad "on"
        MatchDevicePath "/dev/input/event*"
        Driver "libinput"
        Option "ClickMethod" "clickfinger" # other option is "buttonareas"
        Option "DisableWhileTyping" "false"
        Option "AccelSpeed" "0.2"
        #Option "MiddleEmulation" "true" # Only use this with the "buttonareas" clickmethod
EndSection

Others may have similar settings. If possible, it might be nice to read these config files to determine the default option state in the KCM. Then once the user modifies anything, a new user-specific config file gets written that takes priority.

Is there any specific library which could read these files ??

May 15 2019, 4:59 PM · Plasma

May 8 2019

fvogt committed R108:7804eb41d954: Fix crash due to dangling reference (authored by fvogt).
Fix crash due to dangling reference
May 8 2019, 5:15 PM
fvogt closed D21085: Fix crash due to dangling reference.
May 8 2019, 5:15 PM · KWin
fvogt requested review of D21085: Fix crash due to dangling reference.
May 8 2019, 5:10 PM · KWin

May 7 2019

fvogt accepted D21044: Remove kde connect context menu entries when host dies.
May 7 2019, 7:34 AM · Plasma

May 2 2019

fvogt added a comment to T10874: Make pulseaudio-qt available in KDE Connect CI.

Version 1.0.1 is part of Tumbleweed, just zypper in 'cmake(KF5PulseAudioQt)' or pulseaudio-qt-devel.

May 2 2019, 11:21 AM · build.kde.org

Apr 30 2019

fvogt accepted D20925: Also handle muted property on player.

What do you think about adding a x-kde-muted prop and support for it in KDE Connect and the media control applet?

Apr 30 2019, 7:55 PM · Plasma
fvogt added a comment to D20828: Correctly show memory sizes > 4 GiB on 32 bit Linux.

What about Plasma/5.12 and Plasma/5.15?

Apr 30 2019, 8:56 AM · Plasma

Apr 26 2019

fvogt accepted D20819: Make really sure we show an error message in case saving failed.
Apr 26 2019, 11:16 AM · Plasma

Apr 25 2019

fvogt added a comment to D20830: Add hack to unbreak audio playback through pure JS via new Audio().

I'm not sure why this is necessary - during new Audio the created object can't play anything by itself as src is not set.
If just doing createdAudio.paused = false after the removeChild is not enough, is that because the DOM modifications are queued and executed after the audio started playing?

Apr 25 2019, 8:30 PM · Plasma
fvogt requested changes to D20819: Make really sure we show an error message in case saving failed.
Apr 25 2019, 4:17 PM · Plasma
fvogt accepted D20803: Send vivaldi-stable as DesktopEntry.

The RPM as well (luckily, that would've been a mess otherwise)

Apr 25 2019, 9:14 AM · Plasma

Apr 23 2019

fvogt requested changes to D20736: Make artwork selection more standards-compliant and smarter.
Apr 23 2019, 9:15 AM · Plasma

Apr 22 2019

fvogt requested changes to D20736: Make artwork selection more standards-compliant and smarter.
Apr 22 2019, 7:32 PM · Plasma
fvogt accepted D20726: Call into native Media Session browser API if available.
Apr 22 2019, 7:27 PM · Plasma
fvogt accepted D20724: Keep player around when emptied but the website tells us it's actually just paused.
Apr 22 2019, 9:25 AM · Plasma
fvogt added inline comments to D20726: Call into native Media Session browser API if available.
Apr 22 2019, 9:13 AM · Plasma
fvogt added inline comments to D20724: Keep player around when emptied but the website tells us it's actually just paused.
Apr 22 2019, 9:02 AM · Plasma
fvogt added inline comments to D20724: Keep player around when emptied but the website tells us it's actually just paused.
Apr 22 2019, 8:48 AM · Plasma
fvogt accepted D20725: Support "stop" Media Sessions action handler.
Apr 22 2019, 8:46 AM · Plasma

Apr 18 2019

fvogt added a comment to D20659: Copy container in Component::cleanUp before interating.
Apr 18 2019, 6:06 PM · Frameworks
fvogt committed R268:78a711361db3: Copy container in Component::cleanUp before interating (authored by fvogt).
Copy container in Component::cleanUp before interating
Apr 18 2019, 1:16 PM
fvogt closed D20659: Copy container in Component::cleanUp before interating.
Apr 18 2019, 1:16 PM · Frameworks
fvogt retitled D20659: Copy container in Component::cleanUp before interating from Detach container in Component::cleanUp before interating to Copy container in Component::cleanUp before interating.
Apr 18 2019, 12:57 PM · Frameworks
fvogt updated the test plan for D20659: Copy container in Component::cleanUp before interating.
Apr 18 2019, 12:57 PM · Frameworks
fvogt updated the diff for D20659: Copy container in Component::cleanUp before interating.

Use auto (which might actually make it build)

Apr 18 2019, 12:34 PM · Frameworks
fvogt requested review of D20659: Copy container in Component::cleanUp before interating.
Apr 18 2019, 12:32 PM · Frameworks

Apr 16 2019

fvogt accepted D20582: Only consider player gone if really no longer part of the visible DOM.
Apr 16 2019, 9:31 AM · Plasma
fvogt accepted D20596: Set Breeze scrollbars only on HTML tag.

No idea about CSS - as this commit is about breaking the style for most cases though it's probably fine.

Apr 16 2019, 8:49 AM · Plasma
fvogt accepted D20593: Minimize code duplication between node and its children.
Apr 16 2019, 8:48 AM · Plasma

Apr 15 2019

fvogt added a comment to D20582: Only consider player gone if really no longer part of the visible DOM.

I see an opportunity for code decuplication here... Not sure how though - maybe

Apr 15 2019, 6:12 PM · Plasma
fvogt accepted D19857: [DownloadJob] Report total size only if known.
Apr 15 2019, 6:04 PM · Plasma

Apr 13 2019

fvogt accepted D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28.

Code still looks good to me - I can't comment on the cmake parts though.

Apr 13 2019, 11:29 AM · Frameworks

Apr 9 2019

fvogt added a comment to D20407: [Folder View] Improve label crispness.

Sounds good enough to me, let's get this in so openQA won't complain anymore

Apr 9 2019, 4:51 PM · Plasma
fvogt added a comment to D20407: [Folder View] Improve label crispness.

It was actually discovered by openQA as a regression introduced yesterday: https://openqa.opensuse.org/tests/902646#step/finish_desktop/8 (bad) vs https://openqa.opensuse.org/tests/902110#step/finish_desktop/2 (good)
Probably a side effect of the label width change(s).

Apr 9 2019, 2:44 PM · Plasma

Apr 8 2019

fvogt added inline comments to D20227: Remove player from known players list when it disappears.
Apr 8 2019, 8:53 AM · Plasma

Apr 7 2019

fvogt accepted D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28.
Apr 7 2019, 10:07 AM · Frameworks

Apr 6 2019

fvogt added a comment to D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28.

Looking good to me, @bruns: any addiitonal comments?

Apr 6 2019, 12:14 PM · Frameworks
fvogt requested changes to D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28.

Looks good to me otherwise.

Apr 6 2019, 9:59 AM · Frameworks

Apr 5 2019

fvogt committed R98:9aa944ead1a8: Merge branch 'Plasma/5.15' (authored by fvogt).
Merge branch 'Plasma/5.15'
Apr 5 2019, 8:27 AM
fvogt added a comment to D17187: Set the default cursor theme to breeze_cursors.

I just noticed that this change is missing again in 5.15.
Apparently it was reverted (by mistake?) in R98:e1f97ce5 ...

Apr 5 2019, 8:26 AM · Plasma
fvogt committed R98:b00e12ff3926: Set the default cursor theme to breeze_cursors (authored by fvogt).
Set the default cursor theme to breeze_cursors
Apr 5 2019, 8:25 AM
fvogt requested changes to D20259: Fix compilation with LibreSSL.

So libressl's API is mostly openssl < 1.1 with some 1.1 functions sprinkled in? That's annoying.
It might be easier to adjust the check for OSSL_110 to not set it with libressl.

Apr 5 2019, 7:55 AM
fvogt added a comment to D20259: Fix compilation with LibreSSL.

Can you reupload this diff with context?

Apr 5 2019, 6:56 AM

Apr 2 2019

fvogt accepted D20201: Fix build with Qt 4.
Apr 2 2019, 1:20 PM · Plasma
fvogt requested changes to D20201: Fix build with Qt 4.
Apr 2 2019, 1:11 PM · Plasma
fvogt requested changes to D19857: [DownloadJob] Report total size only if known.

Additionally, the bytesReceived > totalAmount case is currently not handled at all, but I'm not sure whether that's something for KJob or here.

Apr 2 2019, 9:50 AM · Plasma
fvogt added a comment to D20135: Enable the k3b helper by default.

Adding security team so we can have a second set of eyes on the helper, i couldn't find anything obviously wrong, but you know how auth helpers are :)

Apr 2 2019, 7:30 AM

Apr 1 2019

fvogt added a comment to D20166: Keep desktoptheme SVG files uncompressed in repo, install svgz.

I'm very much in favor of this, but don't know anything about the cmake magic involved, so can't really say much about that.

Apr 1 2019, 12:39 PM · Frameworks

Mar 31 2019

fvogt added a comment to D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28.

So unless I am mistaken, I feel this is not a great concern.
I would perhaps need to restrict when statx is used even when STATX_BASIC_STATS is defined to when GLIBC is defined as well.

Yes, please do that.

Will do.

And thinking again about the issue, could we have kio compiled with glibc but running on a system with musl for instance ?
If it is possible, then I need to treat this case as you suggested to handle the runtime dependency on glibc.

Mar 31 2019, 11:38 AM · Frameworks
fvogt added a comment to D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28.
In D20096#440830, @pino wrote:

Note that, even if the system supports statx() (so with glibc >= 2.28), you must support systems without it at runtime anyway: for example, if you boot with a kernel older than 4.11 (which IIRC is the version where the syscall was added) then the glibc function will return ENOSYS (IIRC, better check). This can happen for example in containers: you boot a Debian testing container (so with glibc 2.28) on a Debian stable system (with Linux 4.9).

This case is covered by glibc https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/sysdeps/unix/sysv/linux/statx.c
In case the syscall does not exist, glibc provides a generic implementation based on stat.

There are platforms out there which don't use glibc. So I suggest either handling ENOSYS properly by falling back to stat or erroring out if statx is supported but GLIBC not defined.

If the platform does not use glibc, STATX_BASIC_STATS will be false and statx won't be called, the other existing code path will be used.
STATX_BASIC_STATS implies GLIBC and statx availability at least until other C standard libraries support it.

Mar 31 2019, 11:06 AM · Frameworks
fvogt added a comment to D20096: Fill UDSEntry::UDS_CREATION_TIME under linux when glibc >= 2.28.
In D20096#440830, @pino wrote:

Note that, even if the system supports statx() (so with glibc >= 2.28), you must support systems without it at runtime anyway: for example, if you boot with a kernel older than 4.11 (which IIRC is the version where the syscall was added) then the glibc function will return ENOSYS (IIRC, better check). This can happen for example in containers: you boot a Debian testing container (so with glibc 2.28) on a Debian stable system (with Linux 4.9).

This case is covered by glibc https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/sysdeps/unix/sysv/linux/statx.c
In case the syscall does not exist, glibc provides a generic implementation based on stat.

Mar 31 2019, 10:16 AM · Frameworks

Mar 24 2019

fvogt added a comment to T10263: FUSE daemon for accessing KIO Slaves.

@feverfew Sure! I don't know much about GSoC though, I hope that's not an issue.

@fvogt Great, I can tell you more about it on a platform of your choice. What's your preferred method of contact (.e.g email) so we can get a project submitted (deadline is 9th April)?

Mar 24 2019, 8:26 PM

Mar 19 2019

fvogt added a comment to D19821: Fix breeze dialog background with Qt 5.12.2.

This wasn't supposed to land in Plasma 5.15 as well as master?

Mar 19 2019, 1:56 PM · Frameworks
fvogt committed R242:3aba8a7e10e5: Fix breeze dialog background with Qt 5.12.2 (authored by fvogt).
Fix breeze dialog background with Qt 5.12.2
Mar 19 2019, 1:53 PM
fvogt closed D19821: Fix breeze dialog background with Qt 5.12.2.
Mar 19 2019, 1:53 PM · Frameworks
fvogt added a comment to D19821: Fix breeze dialog background with Qt 5.12.2.

I guess it is very likely, but all the same for the record I will request that this results in a 5.56.2 release of plasma-framework.

Mar 19 2019, 1:52 PM · Frameworks