D15721: Make lock on plasmavault icon visible with breeze-dark

ndavis added a comment. 16px F6281723: Screenshot_20180924_005740.png 22px F6281721: Screenshot_20180924_005657.png REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D15721

D15721: Make lock on plasmavault icon visible with breeze-dark

ndavis created this revision. ndavis added a reviewer: VDG. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. ndavis requested review of this revision. REVISION SUMMARY Set lock color on Breeze version to #eff0f1 (light2dark script compatibility) and the lock

D15718: Do not index the path if the path has no execute permissions.

smithjd added a comment. In D15718#330864 , @ngraham wrote: > Making files executable that don't need to be executable is a bad security habit. What if the contents get replaced with something malicious? Suddenly that now-malicious file has

D11880: Add firewall-config and firewall-applet icons

ndavis updated this revision to Diff 42212. ndavis added a comment. Change wall color of firewall-applet-panic to \#4d4d4d (breeze) and \#f2f2f2 (breeze-dark) REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11880?vs=42119=42212 BRANCH

D11880: Add firewall-config and firewall-applet icons

ndavis added a comment. In D11880#330861 , @ngraham wrote: > For the panic mode icon, how about leaving the wall itself black, and only the lock is orange? Like this? This is #4d4d4d (icon grey), the standard color for small breeze

D15718: Do not index the path if the path has no execute permissions.

ngraham added a comment. Making files executable that don't need to be executable is a bad security habit. What if the contents get replaced with something malicious? Suddenly that now-malicious file has execute permissions. --- Conceptually, you are proposing that the rest of the

D15718: Do not index the path if the path has no execute permissions.

smithjd added a comment. In D15718#330845 , @ngraham wrote: > In D15718#330844 , @smithjd wrote: > > > In D15718#330836 , @ngraham wrote: > > > > >

D11880: Add firewall-config and firewall-applet icons

ngraham added a comment. For the panic mode icon, how about leaving the wall itself black, and only the lock is orange? I don't want to totally dominate the conversation here, so I'll step aside for a bit and let others have their say. But I want to let you know that I really

D15718: Do not index the path if the path has no execute permissions.

ngraham added a comment. In D15718#330844 , @smithjd wrote: > In D15718#330836 , @ngraham wrote: > > > Wouldn't this have the effect of un-indexing most files? A quick check of my documents (text,

D15718: Do not index the path if the path has no execute permissions.

smithjd added a comment. In D15718#330836 , @ngraham wrote: > Wouldn't this have the effect of un-indexing most files? A quick check of my documents (text, word processing, excel, etc) reveals that none of them have the execute bit set. As-is,

D15583: [Balooctl] remove directory parent check

bruns added a comment. In D15583#330799 , @smithjd wrote: > Technically, this check isn't actually needed, though it does prevent the user from entering a path more than once. At first glance this looks like it should work: > > if

D15718: Do not index the path if the path has no execute permissions.

ngraham requested changes to this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15718 To: smithjd, ngraham, #baloo Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

bruns added a comment. This is completely wrong ... REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15718 To: smithjd, ngraham Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

bruns added a reviewer: Baloo. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D15718 To: smithjd, ngraham, #baloo Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Wouldn't this have the effect of un-indexing most files? A quick check of my documents (text, word processing, excel, etc) reveals that none of them have the execute bit set.

D15718: Do not index the path if the path has no execute permissions.

smithjd added a comment. Files/folders should not be automatically indexed if the execute bit is unset. Downloads from most if not all popular browsers are not executable by default. Setting a file unexecutable would be used to prevent automatic indexing of files and most importantly

D15718: Do not index the path if the path has no execute permissions.

smithjd created this revision. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. smithjd requested review of this revision. REVISION SUMMARY This will prevent automatic indexing of newly downloaded files and can be used to prevent indexing of

D11880: Add firewall-config and firewall-applet icons

ndavis added a comment. Here's how the checkmark idea looks. I prefer the plain wall since it fits in with the other icons better. 16px F6281345: firewall-applet(check)16.svg 22px F6281342: firewall-applet(check)22.svg

D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

akrutzler added a comment. Thanks for the review. I will address your comments and hopefully have a patch ready in the next days. :) REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D15277 To: akrutzler, elvisangelaccio, ltoscano, hetzenecker, dfaure Cc:

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

smithjd added inline comments. INLINE COMMENTS > taglibwritertest.cpp:60 > > -QCOMPARE(extractedTitle, QStringLiteral("Title1")); > -QCOMPARE(extractedArtist, QStringLiteral("Artist1")); > -QCOMPARE(extractedAlbum, QStringLiteral("Album1")); > +QCOMPARE(extractedTitle,

D11880: Add firewall-config and firewall-applet icons

ndavis added a comment. In D11880#330788 , @ngraham wrote: > I like `firewall-config` and `firewall-applet-error` as they are. > > I think that `firewall-applet` looks maybe a bit too plain at its 22px size. The wall seems to need something.

D15583: [Balooctl] remove directory parent check

smithjd added a comment. Technically, this check isn't actually needed, though it does prevent the user from entering a path more than once. At first glance this looks like it should work: if (folder.startsWith(path)) This doesn't prevent the user from also re-specifying valid

D11880: Add firewall-config and firewall-applet icons

ngraham added a comment. For the benefit of others, here's what they look like now: 22px: F6281196: 22px.png 48px: F6281197: 48px.png I like `firewall-config` and `firewall-applet-error` as they

D8532: [WIP] Restrict file extractor with Seccomp

mgallien added a comment. In D8532#289500 , @davidk wrote: > I was asked in private about the current state of libseccomp integration and why there was no progress in a long time. > The current state is, that I have implemented seccomp support

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

bruns added inline comments. INLINE COMMENTS > svuorela wrote in taglibwritertest.cpp:66 > yeah. given you write and read it, if somehow it gets encoded e.g. as > iso-8859-15 rather than utf8, the euro sign would be encoded as 0xa4 rather > than 0x20ac. > > As you write and read it in the

D8532: [WIP] Restrict file extractor with Seccomp

ngraham added reviewers: Frameworks, smithjd, bruns. ngraham added a comment. +1 for something rather than nothing. No comment on the technical aspect, but I'm adding more reviewers who can hopefully help un-wedge this patch. REPOSITORY R293 Baloo REVISION DETAIL

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

svuorela added inline comments. INLINE COMMENTS > astippich wrote in taglibwritertest.cpp:66 > To be sure I understand correctly, using stringSuffix.toUTF8() is what you > would like to see here? Something along those lines, yes please, to compare with. REPOSITORY R286 KFileMetaData

D15715: remove own implementation of QString to TString conversion for taglibwriter

svuorela added a comment. In D15715#330743 , @astippich wrote: > since I'm still unsure about those things: the q2t function was not declared static, but was never exported. It is still safe to remove, right? Yes. not exported (and not

D15220: implement more basic tags for taglibwriter

astippich added a dependent revision: D15715: remove own implementation of QString to TString conversion for taglibwriter. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15220 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov,

D15715: remove own implementation of QString to TString conversion for taglibwriter

astippich added a dependency: D15220: implement more basic tags for taglibwriter. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15715 To: astippich, mgallien, bruns, svuorela Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,

D15715: remove own implementation of QString to TString conversion for taglibwriter

astippich added a comment. since I'm still unsure about those things: the q2t function was not declared static, but was never exported. It is still safe to remove, right? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15715 To: astippich, mgallien, bruns,

D15715: remove own implementation of QString to TString conversion for taglibwriter

astippich created this revision. astippich added reviewers: mgallien, bruns, svuorela. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY similar to D15614

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

astippich added inline comments. INLINE COMMENTS > svuorela wrote in taglibwritertest.cpp:66 > yeah. given you write and read it, if somehow it gets encoded e.g. as > iso-8859-15 rather than utf8, the euro sign would be encoded as 0xa4 rather > than 0x20ac. > > As you write and read it in the

D15614: remove usage of own TString to QString conversion function

This revision was automatically updated to reflect the committed changes. Closed by commit R286:4c760d4588f3: remove usage of own TString to QString conversion function (authored by astippich). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE

D15615: bump required taglib version to 1.11.1

This revision was automatically updated to reflect the committed changes. Closed by commit R286:26dcd784b45a: bump required taglib version to 1.11.1 (authored by astippich). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15615?vs=41969=42199 REVISION

D15615: bump required taglib version to 1.11.1

astippich retitled this revision from "bump required taglib version" to "bump required taglib version to 1.11.1". REPOSITORY R286 KFileMetaData BRANCH bumb_taglib_version REVISION DETAIL https://phabricator.kde.org/D15615 To: astippich, mgallien, svuorela Cc: kde-frameworks-devel,

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

svuorela added inline comments. INLINE COMMENTS > bruns wrote in taglibwritertest.cpp:66 > ? > The file/tags are written inside this test. yeah. given you write and read it, if somehow it gets encoded e.g. as iso-8859-15 rather than utf8, the euro sign would be encoded as 0xa4 rather than

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

bruns added inline comments. INLINE COMMENTS > svuorela wrote in taglibwritertest.cpp:66 > I suggest checking that the last bytes of all these extracted things is the > actual actual utf8 bytes, so that if someone compiles or saves this file in a > broken encoding that the testing doesn't

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

svuorela added inline comments. INLINE COMMENTS > taglibwritertest.cpp:66 > +QCOMPARE(extractedGenre, QString(QStringLiteral("Genre1") + > stringSuffix)); > +QCOMPARE(extractedComment, QString(QStringLiteral("Comment1") + > stringSuffix)); > I suggest checking that the last bytes of

D15704: increase test coverage of taglibwriter

svuorela added a comment. Looks good to me. A nice and simple way to drastically increase test coverage. INLINE COMMENTS > taglibwriter.cpp:22 > { > QStringList types = { > QStringLiteral("audio/mpeg"), Unrelated. but consider making this static ? REPOSITORY R286

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

astippich created this revision. astippich added reviewers: mgallien, bruns. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY append a unicode character to all test string such that

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

astippich added a dependency: D15704: increase test coverage of taglibwriter. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15714 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D15704: increase test coverage of taglibwriter

astippich added a dependent revision: D15714: add a string suffix to test data and use for unicode testing of taglibwriter. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15704 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov,

KDE CI: Frameworks » kio » kf5-qt5 WindowsMSVCQt5.11 - Build # 28 - Still unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.11/28/ Project: kf5-qt5 WindowsMSVCQt5.11 Date of build: Sun, 23 Sep 2018 16:17:31 + Build duration: 1 hr 23 min and counting JUnit Tests Name:

D15704: increase test coverage of taglibwriter

astippich updated this revision to Diff 42196. astippich added a comment. - don't change test strings for now REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15704?vs=42174=42196 BRANCH taglib_write_mimetypes REVISION DETAIL

D15697: Fix deletion of files from DAV

This revision was automatically updated to reflect the committed changes. Closed by commit R241:7615e3405c30: Fix deletion of files from DAV (authored by dvratil). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15697?vs=42175=42192 REVISION DETAIL

D14530: Fix crash when save a QImage to the eps format file

kossebau added a comment. @zccrs Hi. Does D15405 fix things for you, so is this patch here no longer needed? If so, please close this review request by selecting and submitting the action "Abandon Revision", so it does no longer appear in our "Please

D15704: increase test coverage of taglibwriter

bruns added a comment. In general this looks good, but I would like two changes: 1. Do the conversion to QTest first, and leave out the change for unicode testing (e.g. `Title1` -> `Title €`) 2. Add a third column like "stringsuffix", and then add another test (row) for each format.

D15705: Also raise configuration window when reusing it

broulik created this revision. broulik added reviewers: Plasma, mart. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. broulik requested review of this revision. REVISION SUMMARY It seems `requestActivate` only implies unminimizing but not raising if already

D15697: Fix deletion of files from DAV

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D15697 To: dvratil, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15697: Fix deletion of files from DAV

dvratil edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D15697 To: dvratil, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15697: Fix deletion of files from DAV

dvratil updated this revision to Diff 42175. dvratil added a comment. Improve commit message I thought you were referring to rmdir in the kio slave, but now I realize rmdir() is KIO API, not slave API. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D15220: implement more basic tags for taglibwriter

astippich added a dependent revision: D15704: increase test coverage of taglibwriter. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15220 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham,

D15704: increase test coverage of taglibwriter

astippich added a dependency: D15220: implement more basic tags for taglibwriter. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15704 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,

D15704: increase test coverage of taglibwriter

astippich created this revision. astippich added reviewers: mgallien, bruns. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY increase the test coverage by testing more mimetypes, and

D15697: Fix deletion of files from DAV

dfaure added a comment. Patch looks ok but I'm surprised by the commit log. Doesn't this method also work for directories, even after the patch? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D15697 To: dvratil, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15611: [KCollapsibleGroupBox] Respect style's widget animation duration

elvisangelaccio accepted this revision. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D15611 To: cfeck, #frameworks, ngraham, elvisangelaccio Cc: broulik, ngraham, kde-frameworks-devel, michaelh, bruns

D15645: [WIP] Add scheme selection menu with a "System" entry.

amhndu updated this revision to Diff 42166. amhndu marked 4 inline comments as done. amhndu added a comment. Updated with styling suggestions. REPOSITORY R265 KConfigWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15645?vs=42108=42166 BRANCH system-default REVISION