Re: Review Request 129259: Fix the buffersize in certain situations.

[taro yamada]

> On Oct. 27, 2016, 7:13 p.m., Andreas Hartmetz wrote:
> > Taro, do you have push rights? Otherwise I or somebody else can push it for 
> > you.

Ah, I was misunderstanding the use of reviewboard.
I'll try to push, thank you very much!


- taro


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129259/#review100338
---


On Oct. 27, 2016, 3:02 p.m., taro yamada wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129259/
> ---
> 
> (Updated Oct. 27, 2016, 3:02 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 369275
> https://bugs.kde.org/show_bug.cgi?id=369275
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently, KIO uses lstat to get the buffersize for readlink.
> But in certain situations, it returns inappropriate value.
> 
> For example, "/proc/self" or "/sys/bus/cpu/devices/*" returns its size is 0 , 
> and then readlink fails with EINVAL.(so link won't be shown in kde 
> application.)
> TMSU seems it returns its target's actual filesize insted of the link's 
> filesize itself.
> 
> This patch changes the buffersize to 1024 bytes if it is 0.
> And later truncate it to actual size.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/file.cpp 8b17d31 
> 
> Diff: https://git.reviewboard.kde.org/r/129259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> taro yamada
> 
>

Re: Review Request 129267: [OS X] emulate posix_fallocate()

[Michael Pyne]

> On Oct. 27, 2016, 1:55 a.m., Michael Pyne wrote:
> > src/lib/caching/posix_fallocate_mac.h, line 97
> > 
> >
> > Given that the user has some input into the size of the cache I'd 
> > prefer to have some kind of integer overflow check on offset + len (since 
> > off_t is a signed integer type, this addition is susceptible to undefined 
> > behavior). A potential overflow check is available at 
> > http://stackoverflow.com/a/1514309, and maybe Qt has something as well.
> 
> René J.V. Bertin wrote:
> I think I can safely use `__builtin_saddll_overflow` for the check, 
> possibly using `__has_builtin(__builtin_saddll_overflow)` as a condition for 
> providing the posix_fallocate emulation. The builtin is supposedly available 
> for clang 3.4 and upwards, which means all reasonably recent OS X versions 
> should have it in their system compiler.
> What do you think?

That's fine, and using the __has_builtin guard isn't necessary (I figure you 
know better than I do what build configurations are supported on Mac OS X).


> On Oct. 27, 2016, 1:55 a.m., Michael Pyne wrote:
> > src/lib/caching/posix_fallocate_mac.h, line 108
> > 
> >
> > Given that the ftruncate man page is documented as leaving the current 
> > file offset unchanged, why are we trying to read the old offset and reset 
> > it later? Does ftruncate fail on Mac OS X if the offset is set wrong?
> > 
> > The reason I ask is that we don't check the return value of lseek() 
> > here and while it seems like it should be safe it's at least theoretically 
> > possible to have it return an error.
> 
> René J.V. Bertin wrote:
> That's because to me the man page does not say anything about what it 
> does to file content, when extending a file. The magic with the file position 
> is just to give a bit of additional assurance, by setting the pointer to the 
> specified offset. I would be less shocked if a posix_fallocate() 
> implementation messed up file content after the offset I specified than 
> elsewhere. Then again I'm not entirely sure either how to interpret the 
> meaning of the offset parameter either.
> 
> If you think that's pointless I'll remove the whole thing, and of course 
> it is moot currently because offset is hardcoded to 0 in KCoreAddons.
> Let me know.

Checking the POSIX man page for ftruncate says that it should treat any file 
contents between the old length and extended new length of the file as if it 
were zero-filled, independent of the offset currently set for the file 
descriptor. I think it's safe to assume Mac OS X is POSIX-compatible unless 
they specifically say otherwise in the man page, so I'd say to please remove 
the check to avoid confusion later.


- Michael


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129267/#review100318
---


On Oct. 26, 2016, 6:23 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129267/
> ---
> 
> (Updated Oct. 26, 2016, 6:23 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Mac OS X doesn't have `posix_fallocate()` but as written in the comments, 
> this function can be emulated (at least for simple use with `offset=0`).
> 
> This patch introduces such an emulation, based on `mozilla::fallocation()`. 
> There's so little of that function left that it may be overkill to maintain 
> its original copyright statement but I'll leave that to the legalese experts.
> 
> 
> Diffs
> -
> 
>   src/lib/caching/kshareddatacache_p.h 9fd0bb1 
>   src/lib/caching/posix_fallocate_mac.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/129267/diff/
> 
> 
> Testing
> ---
> 
> Works as intended on OS X 10.9.5 as far as I can tell; original file contents 
> were not overwritten in my testing.
> 
> The autotest succeeds (with an additional debug statement left out of the 
> patch):
> 
> ```
> > kf5-kcoreaddons/work/build/autotests/kshareddatacachetest 
> * Start testing of KSharedDataCacheTest *
> Config: Using QtTest library 5.6.1, Qt 5.6.1 (x86_64-little_endian-lp64 
> shared (dynamic) release build; by Clang 6.0 (clang-600.0.57) (Apple))
> PASS   : KSharedDataCacheTest::initTestCase()
> QWARN  : KSharedDataCacheTest::simpleInsert() void 
> KSharedDataCache::Private::mapSharedMemory() posix_fallocated 5273696 bytes 
> for file "/Users/bertin/.cache/myTestCache.kcache"
> PASS   : KSharedDataCacheTest::simpleInsert()
> PASS   : 

Re: ABI break checking in frameworks

[Albert Astals Cid]

El dijous, 27 d’octubre de 2016, a les 9:20:29 CEST, Jaroslaw Staniek va 
escriure:
> On 26 October 2016 at 23:54, Albert Astals Cid  wrote:
> > Does anyone have other ideas or thoughts on it?
> 
> ​Three things:
> - the Qt project has own solution for the checks, maybe worth looking
> - try to convert the compiler-specific decorated symbols to some uniform
> notation and compare just that
> - add support for ignoring some errors (per line) and check them differently

Let's not reinvent the wheel when there's more than valid projects to do that 
:)

Cheers,
  Albert

Re: Review Request 129259: Fix the buffersize in certain situations.

[Andreas Hartmetz]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129259/#review100338
---


Ship it!




Taro, do you have push rights? Otherwise I or somebody else can push it for you.

- Andreas Hartmetz


On Oct. 27, 2016, 3:02 p.m., taro yamada wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129259/
> ---
> 
> (Updated Oct. 27, 2016, 3:02 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 369275
> https://bugs.kde.org/show_bug.cgi?id=369275
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently, KIO uses lstat to get the buffersize for readlink.
> But in certain situations, it returns inappropriate value.
> 
> For example, "/proc/self" or "/sys/bus/cpu/devices/*" returns its size is 0 , 
> and then readlink fails with EINVAL.(so link won't be shown in kde 
> application.)
> TMSU seems it returns its target's actual filesize insted of the link's 
> filesize itself.
> 
> This patch changes the buffersize to 1024 bytes if it is 0.
> And later truncate it to actual size.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/file.cpp 8b17d31 
> 
> Diff: https://git.reviewboard.kde.org/r/129259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> taro yamada
> 
>

Re: Review Request 129259: Fix the buffersize in certain situations.

[taro yamada]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129259/
---

(Updated Oct. 27, 2016, 3:02 p.m.)


Review request for KDE Frameworks.


Changes
---

changed to use qBound to determine the initial buffersize.


Bugs: 369275
https://bugs.kde.org/show_bug.cgi?id=369275


Repository: kio


Description
---

Currently, KIO uses lstat to get the buffersize for readlink.
But in certain situations, it returns inappropriate value.

For example, "/proc/self" or "/sys/bus/cpu/devices/*" returns its size is 0 , 
and then readlink fails with EINVAL.(so link won't be shown in kde application.)
TMSU seems it returns its target's actual filesize insted of the link's 
filesize itself.

This patch changes the buffersize to 1024 bytes if it is 0.
And later truncate it to actual size.


Diffs (updated)
-

  src/ioslaves/file/file.cpp 8b17d31 

Diff: https://git.reviewboard.kde.org/r/129259/diff/


Testing
---


Thanks,

taro yamada

Re: Review Request 129259: Fix the buffersize in certain situations.

[taro yamada]

> On Oct. 27, 2016, 12:57 p.m., Andreas Hartmetz wrote:
> > src/ioslaves/file/file.cpp, line 791
> > 
> >
> > You can use qMin() here. As is the compiler would complain about the 
> > different types (off_t and int), which can be fixed by changing the type of 
> > initialLimit to off_t.

Thanks!
But I've noticed that st_size could take a negative value.
So I changed to use qBound to determine the initial buffersize.


- taro


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129259/#review100334
---


On Oct. 27, 2016, 4:58 a.m., taro yamada wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129259/
> ---
> 
> (Updated Oct. 27, 2016, 4:58 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 369275
> https://bugs.kde.org/show_bug.cgi?id=369275
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently, KIO uses lstat to get the buffersize for readlink.
> But in certain situations, it returns inappropriate value.
> 
> For example, "/proc/self" or "/sys/bus/cpu/devices/*" returns its size is 0 , 
> and then readlink fails with EINVAL.(so link won't be shown in kde 
> application.)
> TMSU seems it returns its target's actual filesize insted of the link's 
> filesize itself.
> 
> This patch changes the buffersize to 1024 bytes if it is 0.
> And later truncate it to actual size.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/file.cpp 8b17d31 
> 
> Diff: https://git.reviewboard.kde.org/r/129259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> taro yamada
> 
>

[Differential] [Request, 70 lines] D3178: Make kconfig_compiler autotests use the KCONFIG_ADD_KCFG_FILES

[apol (Aleix Pol Gonzalez)]

apol created this revision.
apol added a reviewer: Frameworks.

REVISION SUMMARY
  Instead of having an odd fake of it. Will help some required
  refactorings and already showed some issues, fixed by this patch,
  namely:
  
  - don't use string(regex replace) to extract a string from another
  
  string. in case it doesn't match it will offer the whole content which
  is never what we want.
  
  - messages(ERROR), the correct parameter is FATAL_ERROR, cmake
  
  understands "ERROR" as mere output string
  
  - turn the macro into a function, otherwise 2 calls in the same
  
  subdirectory are dangerous.
  
  CCBUG: 371562

TEST PLAN
  tests still pass, projects that use the macro still build

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D3178

AFFECTED FILES
  KF5ConfigMacros.cmake
  autotests/kconfig_compiler/CMakeLists.txt
  autotests/kconfig_compiler/test1.kcfgc
  autotests/kconfig_compiler/test12.kcfgc
  autotests/kconfig_compiler/test13.kcfgc
  autotests/kconfig_compiler/test9.kcfgc
  autotests/kconfig_compiler/test_qdebugcategory.kcfgc

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: apol, #frameworks

Re: Review Request 129259: Fix the buffersize in certain situations.

[Andreas Hartmetz]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129259/#review100334
---


Ship it!




In any case this is good enough already.


src/ioslaves/file/file.cpp (line 791)


You can use qMin() here. As is the compiler would complain about the 
different types (off_t and int), which can be fixed by changing the type of 
initialLimit to off_t.


- Andreas Hartmetz


On Oct. 27, 2016, 4:58 a.m., taro yamada wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129259/
> ---
> 
> (Updated Oct. 27, 2016, 4:58 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 369275
> https://bugs.kde.org/show_bug.cgi?id=369275
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently, KIO uses lstat to get the buffersize for readlink.
> But in certain situations, it returns inappropriate value.
> 
> For example, "/proc/self" or "/sys/bus/cpu/devices/*" returns its size is 0 , 
> and then readlink fails with EINVAL.(so link won't be shown in kde 
> application.)
> TMSU seems it returns its target's actual filesize insted of the link's 
> filesize itself.
> 
> This patch changes the buffersize to 1024 bytes if it is 0.
> And later truncate it to actual size.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/file.cpp 8b17d31 
> 
> Diff: https://git.reviewboard.kde.org/r/129259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> taro yamada
> 
>

Re: Review Request 129267: [OS X] emulate posix_fallocate()

[René J . V . Bertin]

> On Oct. 27, 2016, 3:55 a.m., Michael Pyne wrote:
> > src/lib/caching/posix_fallocate_mac.h, line 97
> > 
> >
> > Given that the user has some input into the size of the cache I'd 
> > prefer to have some kind of integer overflow check on offset + len (since 
> > off_t is a signed integer type, this addition is susceptible to undefined 
> > behavior). A potential overflow check is available at 
> > http://stackoverflow.com/a/1514309, and maybe Qt has something as well.

I think I can safely use `__builtin_saddll_overflow` for the check, possibly 
using `__has_builtin(__builtin_saddll_overflow)` as a condition for providing 
the posix_fallocate emulation. The builtin is supposedly available for clang 
3.4 and upwards, which means all reasonably recent OS X versions should have it 
in their system compiler.
What do you think?


> On Oct. 27, 2016, 3:55 a.m., Michael Pyne wrote:
> > src/lib/caching/posix_fallocate_mac.h, line 108
> > 
> >
> > Given that the ftruncate man page is documented as leaving the current 
> > file offset unchanged, why are we trying to read the old offset and reset 
> > it later? Does ftruncate fail on Mac OS X if the offset is set wrong?
> > 
> > The reason I ask is that we don't check the return value of lseek() 
> > here and while it seems like it should be safe it's at least theoretically 
> > possible to have it return an error.

That's because to me the man page does not say anything about what it does to 
file content, when extending a file. The magic with the file position is just 
to give a bit of additional assurance, by setting the pointer to the specified 
offset. I would be less shocked if a posix_fallocate() implementation messed up 
file content after the offset I specified than elsewhere. Then again I'm not 
entirely sure either how to interpret the meaning of the offset parameter 
either.

If you think that's pointless I'll remove the whole thing, and of course it is 
moot currently because offset is hardcoded to 0 in KCoreAddons.
Let me know.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129267/#review100318
---


On Oct. 26, 2016, 8:23 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129267/
> ---
> 
> (Updated Oct. 26, 2016, 8:23 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Mac OS X doesn't have `posix_fallocate()` but as written in the comments, 
> this function can be emulated (at least for simple use with `offset=0`).
> 
> This patch introduces such an emulation, based on `mozilla::fallocation()`. 
> There's so little of that function left that it may be overkill to maintain 
> its original copyright statement but I'll leave that to the legalese experts.
> 
> 
> Diffs
> -
> 
>   src/lib/caching/kshareddatacache_p.h 9fd0bb1 
>   src/lib/caching/posix_fallocate_mac.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/129267/diff/
> 
> 
> Testing
> ---
> 
> Works as intended on OS X 10.9.5 as far as I can tell; original file contents 
> were not overwritten in my testing.
> 
> The autotest succeeds (with an additional debug statement left out of the 
> patch):
> 
> ```
> > kf5-kcoreaddons/work/build/autotests/kshareddatacachetest 
> * Start testing of KSharedDataCacheTest *
> Config: Using QtTest library 5.6.1, Qt 5.6.1 (x86_64-little_endian-lp64 
> shared (dynamic) release build; by Clang 6.0 (clang-600.0.57) (Apple))
> PASS   : KSharedDataCacheTest::initTestCase()
> QWARN  : KSharedDataCacheTest::simpleInsert() void 
> KSharedDataCache::Private::mapSharedMemory() posix_fallocated 5273696 bytes 
> for file "/Users/bertin/.cache/myTestCache.kcache"
> PASS   : KSharedDataCacheTest::simpleInsert()
> PASS   : KSharedDataCacheTest::cleanupTestCase()
> Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted
> * Finished testing of KSharedDataCacheTest *
> ```
> 
> 
> File Attachments
> 
> 
> a simple commandline test app
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/10/26/e0af195f-b484-4647-b211-cc78b5b0584b__posix_fallocate.c
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Review Request 129261: Hide the "Show Menu Bar" action if all the menubars are native

[Kai Uwe Broulik]

> On Okt. 27, 2016, 10:38 vorm., Kai Uwe Broulik wrote:
> > The problem with isNativeMenuBar() is that it returns true if the window 
> > *may* be a native menu bar, for example when the platform theme removes the 
> > AA_DontUseNativeMenuBar qApp flag. This does not mean that the platform 
> > theme actually created a native menu bar for the window - which ours does 
> > not if no global menu service is available.
> > 
> > Changing it from checking whether platformMenuBar() returns something fixes 
> > this for me and the entry behaves properly, being hidden when global menu 
> > is available, and being visible if not.

There's a Qt patch that fixes this 
https://code.qt.io/cgit/qt/qtbase.git/commit/?id=835d7cf54328bdd93d58bb64ed96a9c322580aea


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129261/#review100331
---


On Okt. 25, 2016, 10:15 nachm., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129261/
> ---
> 
> (Updated Okt. 25, 2016, 10:15 nachm.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> ---
> 
> Some applications have a "Show Menu Bar" action that is a bit silly on 
> systems where the menubar is part of the shell (for example Unity 7).
> 
> This patch attempts to fix it by iterating all the main windows when they are 
> shown and if all the menubars of all mainwindows are native, then hides the 
> show menu bar action (basically erasing it from existence).
> 
> It's not the nicest of the codes and probably has some edge cases but works 
> on the general case so i think it's worth the effort.
> 
> 
> Diffs
> -
> 
>   src/kstandardaction.cpp 89d011e 
> 
> Diff: https://git.reviewboard.kde.org/r/129261/diff/
> 
> 
> Testing
> ---
> 
> Tried konsole, kate and dolphin under Unity 7 on Ubuntu 16.10
> 
> konsole and kate work fine (i.e. the action is gone from the menus and all is 
> good)
> 
> dolphin is not 100% "perfectly behabed" (i.e. the "control" toolbar item is 
> supposed to not be shown when menubars are shown and in this case it's shown) 
> but it's not a regression and imho it's the dolphin code being a bit weird (i 
> can propose a patch for it if this gets accepted)
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

Re: Review Request 129261: Hide the "Show Menu Bar" action if all the menubars are native

[Kai Uwe Broulik]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129261/#review100331
---



The problem with isNativeMenuBar() is that it returns true if the window *may* 
be a native menu bar, for example when the platform theme removes the 
AA_DontUseNativeMenuBar qApp flag. This does not mean that the platform theme 
actually created a native menu bar for the window - which ours does not if no 
global menu service is available.

Changing it from checking whether platformMenuBar() returns something fixes 
this for me and the entry behaves properly, being hidden when global menu is 
available, and being visible if not.

- Kai Uwe Broulik


On Okt. 25, 2016, 10:15 nachm., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129261/
> ---
> 
> (Updated Okt. 25, 2016, 10:15 nachm.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> ---
> 
> Some applications have a "Show Menu Bar" action that is a bit silly on 
> systems where the menubar is part of the shell (for example Unity 7).
> 
> This patch attempts to fix it by iterating all the main windows when they are 
> shown and if all the menubars of all mainwindows are native, then hides the 
> show menu bar action (basically erasing it from existence).
> 
> It's not the nicest of the codes and probably has some edge cases but works 
> on the general case so i think it's worth the effort.
> 
> 
> Diffs
> -
> 
>   src/kstandardaction.cpp 89d011e 
> 
> Diff: https://git.reviewboard.kde.org/r/129261/diff/
> 
> 
> Testing
> ---
> 
> Tried konsole, kate and dolphin under Unity 7 on Ubuntu 16.10
> 
> konsole and kate work fine (i.e. the action is gone from the menus and all is 
> good)
> 
> dolphin is not 100% "perfectly behabed" (i.e. the "control" toolbar item is 
> supposed to not be shown when menubars are shown and in this case it's shown) 
> but it's not a regression and imho it's the dolphin code being a bit weird (i 
> can propose a patch for it if this gets accepted)
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

Re: Review Request 126028: Add support for desktopFileName to NETWinInfo

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126028/
---

(Updated Oct. 27, 2016, 9:33 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, kwin and David Faure.


Changes
---

Submitted with commit e333a13fc52c28aac3c0d28cd5b85e16428fcff7 by Martin 
Gräßlin to branch master.


Repository: kwindowsystem


Description
---

Implementation for the new hint _NET_WM_DESKTOP_FILE, see [1].
Till it's fully standardized going with a KDE prefix, so it's
_KDE_NET_WM_DESKTOP_FILE. Once it's specified the atom name can
be exchanged.

[1] https://mail.gnome.org/archives/wm-spec-list/2015-November/msg0.html


Diffs
-

  autotests/netwininfotestclient.cpp 59a3980ff589fc3de9e479905c191bcbf1747644 
  src/netwm_def.h 0938e2b28f6b0d425a16748b52a5b8e4704d8af6 
  src/platforms/xcb/atoms_p.h b5a6e7efc98ff8033c0854041428a7d4b52ffc93 
  src/platforms/xcb/netwm.h 220340bf0e96d2a186b72e118b601471529aeabf 
  src/platforms/xcb/netwm.cpp 2335c297bb627065ca9b7e691290bfbdc64bccc3 
  src/platforms/xcb/netwm_p.h e0645bbfd2c2061d9201fe34160c6201d89f4a89 

Diff: https://git.reviewboard.kde.org/r/126028/diff/


Testing
---

tested with a modified kcmshell5 and a modified desktop file.


Thanks,

Martin Gräßlin

Re: ABI break checking in frameworks

[Jaroslaw Staniek]

On 26 October 2016 at 23:54, Albert Astals Cid  wrote:

>
>
>
> Does anyone have other ideas or thoughts on it?
>
>
​Three things:
- the Qt project has own solution for the checks, maybe worth looking
- try to convert the compiler-specific decorated symbols to some uniform
notation and compare just that
- add support for ignoring some errors (per line) and check them differently

-- 
regards, Jaroslaw Staniek

KDE:
: A world-wide network of software engineers, artists, writers, translators
: and facilitators committed to Free Software development - http://kde.org
Calligra Suite:
: A graphic art and office suite - http://calligra.org
Kexi:
: A visual database apps builder - http://calligra.org/kexi
Qt Certified Specialist:
: http://www.linkedin.com/in/jstaniek

Re: Review Request 129262: Properly finish DropJobs when triggered is not emitted

[David Faure]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129262/#review100324
---


Ship it!




Thanks for the fix. Given that there is no "canceled" signal from QMenu, this 
looks like the only solution indeed.

- David Faure


On Oct. 26, 2016, 9:50 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129262/
> ---
> 
> (Updated Oct. 26, 2016, 9:50 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 363936
> https://bugs.kde.org/show_bug.cgi?id=363936
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> If the user closes a DndPopupMenu without triggering an action, the DropJob 
> never emits the result signal. This results in Dolphin hangs (see bug 363936).
> 
> This patch uses a boolean flag to track whether an action has been triggered 
> in the job's popup menu.
> If the flag is false and the menu gets deleted, we need to manually emit the 
> result signal.
> 
> QMenu emits `aboutToHide` before `triggered`, so I had to use a QTimer as 
> workaround. I'm not sure if there is a better way to do this...
> 
> 
> Diffs
> -
> 
>   src/widgets/dropjob.cpp f033bfb5318624836d8b83c6783cf998990dcc02 
> 
> Diff: https://git.reviewboard.kde.org/r/129262/diff/
> 
> 
> Testing
> ---
> 
> Make sure https://bugs.kde.org/show_bug.cgi?id=363936 is fixed.
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>