Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

[Kåre Särs]

> On Feb. 14, 2016, 11:11 p.m., Aleix Pol Gonzalez wrote:
> > src/platformtheme/kdeplatformfiledialoghelper.cpp, line 192
> > 
> >
> > Wouldn't it be a good practice to assume people are doing the right 
> > thing and not passing something that isn't a folder?
> > 
> > Do other implementations make this check? I doubt it.

>From the QFileDialog manual:
getOpenFileUrls(): "The function is used similarly to 
QFileDialog::getOpenFileNames(). In particular parent, caption, dir, filter, 
selectedFilter and options are used in the exact same way."

And in getOpenFileNames(): "The file dialog's working directory will be set to 
dir. If dir includes a file name, the file will be selected."

Unfortunately this does not work for remote URLs as Qt can't know if it points 
to a file or directory. That is why we need to check it here.


- Kåre


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


On Feb. 14, 2016, 6:25 a.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
> 
> (Updated Feb. 14, 2016, 6:25 a.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and David Faure.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> Qt does not know if the remote URL points to a file or directory. that is why 
> options()->initialDirectory() returns the full URL even if it is a file.
> 
> 
> This fix is a bit like Alex Richardson workaround in KIO 
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in 
> stead (I did not see his/Your KIO fix before now...)
> 
> I check the remote url in setDirectory() because setDirectoy() is called from 
> two places.
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 
> 
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
> 
> 
> Testing
> ---
> 
> Kate now happily opens local and remote folders :)
> 
> 
> File Attachments
> 
> 
> firefox.desktop
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/01/25/60c21962-396e-468e-add9-e7112c49d7ba__firefox.desktop
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

[Kåre Särs]

> On Feb. 14, 2016, 11:12 p.m., Aleix Pol Gonzalez wrote:
> > src/platformtheme/kdeplatformfiledialoghelper.cpp, line 198
> > 
> >
> > We shouldn't be calling sync API there.

That is true :( 

I have been debugging this a bit and it is not very straight forward... When we 
open the QFileDialog we actually get 3 calls to setDirectory() I have been 
thinking about figuring out why it is called so many times but I have not been 
able to pinpoint what would be needed to get it to only set the directory and 
file once.

KDirSelectDialog is using job->exec() in 4 places and the 
KDirOperator::setUrl() would also use job->exec()

but maybe that is the place it should be fixed?

in KDirOperator::setUrl() (kio/src/filewidgets/kdiroperator.cpp line 1005) we 
have:

if (!Private::isReadable(newurl)) {
// maybe newurl is a file? check its parent directory

And here we go on to use job->exec() and friends to check if the URL is 
readable and a directory. Except that Private::isReadable() always returns true 
for remote URLs...

What is the proper solution here?


- Kåre


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


On Feb. 14, 2016, 6:25 a.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
> 
> (Updated Feb. 14, 2016, 6:25 a.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and David Faure.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> Qt does not know if the remote URL points to a file or directory. that is why 
> options()->initialDirectory() returns the full URL even if it is a file.
> 
> 
> This fix is a bit like Alex Richardson workaround in KIO 
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in 
> stead (I did not see his/Your KIO fix before now...)
> 
> I check the remote url in setDirectory() because setDirectoy() is called from 
> two places.
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 
> 
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
> 
> 
> Testing
> ---
> 
> Kate now happily opens local and remote folders :)
> 
> 
> File Attachments
> 
> 
> firefox.desktop
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/01/25/60c21962-396e-468e-add9-e7112c49d7ba__firefox.desktop
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126291: initial implementation of a platform plugin for OS X (WIP)

[Martin Gräßlin]

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



Thanks for contributing the code, I'm very happy to see this happening! Sorry, 
that I cannot provide a good review as I'm not really into the OSX API, thus my 
review here might look a little bit nitpicky.

General comment: maybe rename the directory from "osx" to "cocoa"? After all 
it's more about the windowing system than the operating system.


src/platforms/osx/kwindowsystem_macobjc.mm (line 89)


Why is the define needed? Doesn't it make more sense to just not build the 
OSX backend if COCOA is disabled?



src/platforms/osx/kwindowsystem_macobjc.mm (lines 111 - 113)


please no commented code



src/platforms/osx/kwindowsystem_macobjc.mm (line 175)


Please use categorized logging. I suggest adding a new category for the OSX 
backend.



src/platforms/osx/kwindowsystem_macobjc.mm (lines 302 - 303)


then please remove the code



src/platforms/osx/kwindowsystem_macobjc.mm (lines 331 - 342)


why support setting the current desktop if the number of desktops is 
hardcoded to 1?



src/platforms/osx/kwindowsystem_macobjc.mm (line 446)


what's experimental window tracking?

And that ifdef if enabled would not compile



src/platforms/osx/kwindowsystem_macobjc.mm (line 447)


QObject()


- Martin Gräßlin


On Feb. 14, 2016, 5:44 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126291/
> ---
> 
> (Updated Feb. 14, 2016, 5:44 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KWindowSystem has been lacking a platform plugin for OS X. This RR presents a 
> "backport" of the modified KDE4 KWindowSystem implementation that has been 
> used in the MacPorts kdelibs4 port for the last 2 or 3 (or more) years.
> 
> I have made some initial steps to remove deprecated Carbon API calls, but 
> this is clearly a work in progress.
> 
> Open questions include
> - is there any justification to run an event handler (or Cocoa observer) to 
> keep track of running applications, possibly even listing all their open 
> windows?
> - is there any use for the Qt event listener framework (cf. the 
> NETEventFilter in the X11 plugin)? I haven't yet had time to try to figure 
> out what this "could be good for", and am very open to suggestions in this 
> departments.
> - one application for such an event filter would be a listener that catches 
> the opening and closing of all windows by the running process, and keeps 
> track of their `WId`s. A new method could then be added (to `KWindowInfo`?) 
> to distinguish `WId`s created by the running application from "foreign" ones 
> (useful also on Wayland and MS Windows).
> 
> `KWindowSystem::setMainWindow` should become a front for payload provided by 
> the plugins. I'll leave that to the regular/official maintainer(s) of this 
> framework.
> 
> This code could probably do with *lots* of comments; I'll try to add them as 
> questions about this or that bit of code arise.
> 
> 
> Diffs
> -
> 
>   src/kwindowsystem.cpp 407a67d 
>   src/platforms/osx/CMakeLists.txt 4fc3347 
>   src/platforms/osx/cocoa.json PRE-CREATION 
>   src/platforms/osx/kkeyserver.cpp 3ddb921 
>   src/platforms/osx/kwindowinfo.cpp e8555bb 
>   src/platforms/osx/kwindowinfo.mm PRE-CREATION 
>   src/platforms/osx/kwindowinfo_mac_p.h c8f307e 
>   src/platforms/osx/kwindowinfo_p_cocoa.h PRE-CREATION 
>   src/platforms/osx/kwindowsystem.cpp 1758829 
>   src/platforms/osx/kwindowsystem_mac_p.h PRE-CREATION 
>   src/platforms/osx/kwindowsystem_macobjc.mm PRE-CREATION 
>   src/platforms/osx/kwindowsystem_p_cocoa.h PRE-CREATION 
>   src/platforms/osx/plugin.h PRE-CREATION 
>   src/platforms/osx/plugin.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126291/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9.5 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127024: Don't filter names if we have mimetypes

[Aleix Pol Gonzalez]

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

(Updated Feb. 15, 2016, 12:31 a.m.)


Review request for KDE Frameworks and Plasma.


Changes
---

Address issue as pointed by David.


Repository: frameworkintegration


Description
---

setMimeTypeFilters will call setNameFilter anyway, so we're not missing any 
entries, on the other hand setMimeFilter will do some nice things using the 
extra knowledge.


Diffs (updated)
-

  src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 

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


Testing
---

Tested it while porting okular.


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127044: Show a warning if there's an error in the Engine

[Aleix Pol Gonzalez]

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

(Updated Feb. 14, 2016, 11:28 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Jeremy Whiting.


Changes
---

Submitted with commit 06089b0edb7c7d7e3f8566c8eb01e64d254a167f by Aleix Pol to 
branch master.


Repository: knewstuff


Description
---

So that we can know what's going on if things don't go smoothly.

Note the signal used to be emitted but nobody was listening to it.


Diffs
-

  src/downloadmanager.h 1990ef1 
  src/downloadmanager.cpp c99c524 

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


Testing
---


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127044: Show a warning if there's an error in the Engine

[Aleix Pol Gonzalez]

> On Feb. 13, 2016, 7:31 p.m., David Faure wrote:
> > That's a start, for debugging. The question is, should the user be notified 
> > too, in some way?

True, I'll leave that up to the maintainer, especially given it might mean 
changes in the UI.


- Aleix


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


On Feb. 15, 2016, 12:28 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127044/
> ---
> 
> (Updated Feb. 15, 2016, 12:28 a.m.)
> 
> 
> Review request for KDE Frameworks and Jeremy Whiting.
> 
> 
> Repository: knewstuff
> 
> 
> Description
> ---
> 
> So that we can know what's going on if things don't go smoothly.
> 
> Note the signal used to be emitted but nobody was listening to it.
> 
> 
> Diffs
> -
> 
>   src/downloadmanager.h 1990ef1 
>   src/downloadmanager.cpp c99c524 
> 
> Diff: https://git.reviewboard.kde.org/r/127044/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126480: Fix assorted memory leaks and undefined accesses

[Michael Pyne]

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

(Updated Feb. 14, 2016, 8:06 p.m.)


Review request for KDE Frameworks, David Faure, Martin Tobias Holmedahl 
Sandsmark, and Patrick Spendrin.


Changes
---

Re-uploading previous diff, the RB upgrade seems to have garbled the existing 
diff somehow. No code changes since the RR was opened.


Repository: khtml


Description
---

This is a compendium of proposed fixes to the outstanding "High Impact" issues 
Coverity has reported against KHTML, and which I in my unilateral and infinite 
wisdom have judged are actually valid (more opinions are welcome! It should be 
possible to register for Coverity access to KDE runs if you ask from your 
kde.org address).

Coverity issues issues include CID #s: 257995, 257996, 259759, 257994, 257927, 
257928, 1019703, 1019704, 1340892, 1340908

Fix-specific notes:

A few fixes for leaked auxiliary structs used to calculate a return value 
simply used a QScopedPointer, unless a mere "delete foo" was all that was 
required.

CID 259759, tiled pixmap cache misusage: QCache can in theory delete an object 
you insert as soon as you insert it, so we must guard against that possibility. 
If re-entrancy or concurrent use of the cache were possible then we would also 
need to guard against objects obtained from the cache being deleted, but I 
don't think that's possible so I didn't add sanity checking for that.

CID 257994, leaked font face during CSS parsing: I'm not completely sure this 
is possible but if it is, malicious web sites could induce possibly-large 
memory leaks. The leak would occur if a font with an empty/invalid name leaked 
into this routine (I think it would be even more restrictive than that, the 
leak would only be possible if no other font family were valid).

CID 257928, leak (mis-parse?) during CSS background property parsing: In 
essence we manually parse a list of value pairs, which are read into 
currValue{,2} and then migrated from currValue{,2} into value{,2} (a single 
value) and values{,2} (a list) in turn. The end of the procedure assumes that 
value{,2} are either both single values or both value lists, but the code path 
might result in a value list for the first with only a single value for the 
second. That single value for the second part of the pair would then be leaked 
(since the values2 list is empty at this point).

CID 1019703, 1019704, DTD Tag IDs: Potentially serious? At some point we added 
a tag type for "COMMENT" (which I would think the parser would eliminate from 
the AST, but maybe that is no longer the case?). However we didn't expand out 
some auxiliary data vectors we use for the tags, which Coverity flagged as a 
potential invalid memory access.


Diffs (updated)
-

  src/css/css_webfont.cpp 6754a30 
  src/css/cssparser.cpp 112c867 
  src/ecma/kjs_html.cpp cc3c08a 
  src/editing/htmlediting_impl.cpp d56c4a8 
  src/html/dtd.cpp 71cad5c 
  src/rendering/render_object.cpp 06dba1a 
  src/xml/dom_docimpl.cpp 4f0be5c 

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


Testing
---

Everything compiles, clicking around in Konq on various websites and opening 
local XML files seems to work fine. I can't get CMake to build the autotests 
however (though there aren't very many anyways).


Thanks,

Michael Pyne

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127048: Restrict _nl_msg_cat_cntr use to GNU gettext implentations.

[Andreas Cord-Landwehr]

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

(Updated Feb. 14, 2016, 8:21 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.


Changes
---

Submitted with commit cdfad77bc9fc21aee1b0d4ca3ea3589552ed03c8 by Andreas 
Cord-Landwehr to branch master.


Repository: ki18n


Description
---

On Android, we do not have libintl but only libintl-lite, which does
not provide _nl_msg_cat_cntr. Actually, _nl_msg_cat_cntr is a GNU
gettext specific tool to prevent wrong localization after runtime language
changes. Thus, it is tied to the specific GNU gettext implementation.


Diffs
-

  src/kcatalog.cpp 095a6be 

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


Testing
---

Tested compilation on Android and Linux.


Thanks,

Andreas Cord-Landwehr

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127077: kio_http: read and discard body after a 404 with errorPage=false.

[Milian Wolff]

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


Fix it, then Ship it!





autotests/http_jobtest.cpp (line 71)


what fails? can you `QEXPECT_FAIL` it? or did you fix it already (I assume 
that is the case)


- Milian Wolff


On Feb. 14, 2016, 9:57 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127077/
> ---
> 
> (Updated Feb. 14, 2016, 9:57 p.m.)
> 
> 
> Review request for KDE Frameworks, Dawit Alemayehu, Andreas Hartmetz, and 
> Rolf Eike Beer.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> kio_http: read and discard body after a 404 with errorPage=false.
> 
> When getting a 404 error with some content, the job succeeds and returns 
> content
> (good for webbrowsers).
> The metadata "errorPage" can be set to false so that the job fails instead
> (useful for other cases, like favicon download, file copy etc.)
> However the rest of the headers, as well as the body must still be read and
> discarded, otherwise they clobber the next request (kio_http then starts
> parsing in the middle of some headers and says "DO NOT WANT").
> 
> This is not a porting bug, I could reproduce it with kdelibs4 too.
> 
> 
> Diffs
> -
> 
>   autotests/http_jobtest.cpp PRE-CREATION 
>   src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 
> 
> Diff: https://git.reviewboard.kde.org/r/127077/diff/
> 
> 
> Testing
> ---
> 
> Unittest.
> 
> Please review this patch carefully, I don't have much experience with this 
> code in kio_http, and the potential for regressions in cases that I didn't 
> test is likely high.
> 
> 
> Thanks,
> 
> David Faure
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

[Aleix Pol Gonzalez]

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




src/platformtheme/kdeplatformfiledialoghelper.cpp (line 192)


Wouldn't it be a good practice to assume people are doing the right thing 
and not passing something that isn't a folder?

Do other implementations make this check? I doubt it.


- Aleix Pol Gonzalez


On Feb. 14, 2016, 7:25 a.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
> 
> (Updated Feb. 14, 2016, 7:25 a.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and David Faure.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> Qt does not know if the remote URL points to a file or directory. that is why 
> options()->initialDirectory() returns the full URL even if it is a file.
> 
> 
> This fix is a bit like Alex Richardson workaround in KIO 
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in 
> stead (I did not see his/Your KIO fix before now...)
> 
> I check the remote url in setDirectory() because setDirectoy() is called from 
> two places.
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 
> 
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
> 
> 
> Testing
> ---
> 
> Kate now happily opens local and remote folders :)
> 
> 
> File Attachments
> 
> 
> firefox.desktop
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/01/25/60c21962-396e-468e-add9-e7112c49d7ba__firefox.desktop
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

[Aleix Pol Gonzalez]

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




src/platformtheme/kdeplatformfiledialoghelper.cpp (line 198)


We shouldn't be calling sync API there.


- Aleix Pol Gonzalez


On Feb. 14, 2016, 7:25 a.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
> 
> (Updated Feb. 14, 2016, 7:25 a.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and David Faure.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> Qt does not know if the remote URL points to a file or directory. that is why 
> options()->initialDirectory() returns the full URL even if it is a file.
> 
> 
> This fix is a bit like Alex Richardson workaround in KIO 
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in 
> stead (I did not see his/Your KIO fix before now...)
> 
> I check the remote url in setDirectory() because setDirectoy() is called from 
> two places.
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 
> 
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
> 
> 
> Testing
> ---
> 
> Kate now happily opens local and remote folders :)
> 
> 
> File Attachments
> 
> 
> firefox.desktop
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/01/25/60c21962-396e-468e-add9-e7112c49d7ba__firefox.desktop
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127076: kio_http: fix mimetype determination when URL ends with '/'.

[Milian Wolff]

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


Fix it, then Ship it!





autotests/httpserver_p.h (line 121)


move to .cpp, also above and below?



autotests/httpserver_p.h (line 141)


const (or static?)



autotests/httpserver_p.cpp (line 95)


replace with category based logging?


- Milian Wolff


On Feb. 14, 2016, 6:50 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127076/
> ---
> 
> (Updated Feb. 14, 2016, 6:50 p.m.)
> 
> 
> Review request for KDE Frameworks, Dawit Alemayehu and Andreas Hartmetz.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> When no Content-Type is sent by the server, we determine mimetype from
> the URL and the contents. In the switch from KMimeType (which
> took the fileName of the URL) and QMimeDatabase (which takes the full path),
> we hit a difference: if the path ends with '/' then QMimeDatabase
> assumes it's a directory, which isn't the case over HTTP.
> So remove the trailing slash.
> 
> This commit introduces a test harness for kio_http: a basic HTTP server
> running in a separate thread, which I wrote for KDSoap (LGPL).
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 107263820136c599df84c7beb06a29a1c52898ae 
>   autotests/http_jobtest.cpp PRE-CREATION 
>   autotests/httpserver_p.h PRE-CREATION 
>   autotests/httpserver_p.cpp PRE-CREATION 
>   src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 
> 
> Diff: https://git.reviewboard.kde.org/r/127076/diff/
> 
> 
> Testing
> ---
> 
> Unit test + watching kio_http debug output in my current work on 
> keditbookmarks favicon support.
> 
> The missing Content-Type is due to another bug, but that's for the next RR.
> 
> 
> Thanks,
> 
> David Faure
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127077: kio_http: read and discard body after a 404 with errorPage=false.

[David Faure]

> On Feb. 14, 2016, 9:58 p.m., Milian Wolff wrote:
> > autotests/http_jobtest.cpp, line 71
> > 
> >
> > what fails? can you `QEXPECT_FAIL` it? or did you fix it already (I 
> > assume that is the case)

LOL I forgot to remove that comment. This commit is the fix for it.

Thanks for the review, but I admit I would really like a kio_http contributor 
to look at it, I don't feel at ease with the change being correct in 100% of 
the cases.

Of course if we get any regressions, from now on we can actually add 
unittests


- David


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


On Feb. 14, 2016, 9:57 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127077/
> ---
> 
> (Updated Feb. 14, 2016, 9:57 p.m.)
> 
> 
> Review request for KDE Frameworks, Dawit Alemayehu, Andreas Hartmetz, and 
> Rolf Eike Beer.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> kio_http: read and discard body after a 404 with errorPage=false.
> 
> When getting a 404 error with some content, the job succeeds and returns 
> content
> (good for webbrowsers).
> The metadata "errorPage" can be set to false so that the job fails instead
> (useful for other cases, like favicon download, file copy etc.)
> However the rest of the headers, as well as the body must still be read and
> discarded, otherwise they clobber the next request (kio_http then starts
> parsing in the middle of some headers and says "DO NOT WANT").
> 
> This is not a porting bug, I could reproduce it with kdelibs4 too.
> 
> 
> Diffs
> -
> 
>   autotests/http_jobtest.cpp PRE-CREATION 
>   src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 
> 
> Diff: https://git.reviewboard.kde.org/r/127077/diff/
> 
> 
> Testing
> ---
> 
> Unittest.
> 
> Please review this patch carefully, I don't have much experience with this 
> code in kio_http, and the potential for regressions in cases that I didn't 
> test is likely high.
> 
> 
> Thanks,
> 
> David Faure
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127077: kio_http: read and discard body after a 404 with errorPage=false.

[David Faure]

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

(Updated Feb. 14, 2016, 9:57 p.m.)


Review request for KDE Frameworks, Dawit Alemayehu, Andreas Hartmetz, and Rolf 
Eike Beer.


Changes
---

updated title


Summary (updated)
-

kio_http: read and discard body after a 404 with errorPage=false.


Repository: kio


Description
---

kio_http: read and discard body after a 404 with errorPage=false.

When getting a 404 error with some content, the job succeeds and returns content
(good for webbrowsers).
The metadata "errorPage" can be set to false so that the job fails instead
(useful for other cases, like favicon download, file copy etc.)
However the rest of the headers, as well as the body must still be read and
discarded, otherwise they clobber the next request (kio_http then starts
parsing in the middle of some headers and says "DO NOT WANT").

This is not a porting bug, I could reproduce it with kdelibs4 too.


Diffs
-

  autotests/http_jobtest.cpp PRE-CREATION 
  src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 

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


Testing
---

Unittest.

Please review this patch carefully, I don't have much experience with this code 
in kio_http, and the potential for regressions in cases that I didn't test is 
likely high.


Thanks,

David Faure

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127077: kio_http: read and discard body after a 404 with errorPage=false.

[David Faure]

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

(Updated Feb. 14, 2016, 10:23 p.m.)


Review request for KDE Frameworks, Dawit Alemayehu, Andreas Hartmetz, and Rolf 
Eike Beer.


Changes
---

removed outdated comment


Repository: kio


Description
---

kio_http: read and discard body after a 404 with errorPage=false.

When getting a 404 error with some content, the job succeeds and returns content
(good for webbrowsers).
The metadata "errorPage" can be set to false so that the job fails instead
(useful for other cases, like favicon download, file copy etc.)
However the rest of the headers, as well as the body must still be read and
discarded, otherwise they clobber the next request (kio_http then starts
parsing in the middle of some headers and says "DO NOT WANT").

This is not a porting bug, I could reproduce it with kdelibs4 too.


Diffs (updated)
-

  autotests/http_jobtest.cpp PRE-CREATION 
  src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 

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


Testing
---

Unittest.

Please review this patch carefully, I don't have much experience with this code 
in kio_http, and the potential for regressions in cases that I didn't test is 
likely high.


Thanks,

David Faure

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127076: kio_http: fix mimetype determination when URL ends with '/'.

[David Faure]

> On Feb. 14, 2016, 10:02 p.m., Milian Wolff wrote:
> > autotests/httpserver_p.cpp, line 95
> > 
> >
> > replace with category based logging?

I'd like to keep the diff not _too_ different from the code in kdsoap, which 
has to still compile with Qt4.
[This particular line uses qgetenv there obviously, but I mean the whole if 
(doDebug) stuff makes the files more similar]


- David


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


On Feb. 14, 2016, 6:50 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127076/
> ---
> 
> (Updated Feb. 14, 2016, 6:50 p.m.)
> 
> 
> Review request for KDE Frameworks, Dawit Alemayehu and Andreas Hartmetz.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> When no Content-Type is sent by the server, we determine mimetype from
> the URL and the contents. In the switch from KMimeType (which
> took the fileName of the URL) and QMimeDatabase (which takes the full path),
> we hit a difference: if the path ends with '/' then QMimeDatabase
> assumes it's a directory, which isn't the case over HTTP.
> So remove the trailing slash.
> 
> This commit introduces a test harness for kio_http: a basic HTTP server
> running in a separate thread, which I wrote for KDSoap (LGPL).
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 107263820136c599df84c7beb06a29a1c52898ae 
>   autotests/http_jobtest.cpp PRE-CREATION 
>   autotests/httpserver_p.h PRE-CREATION 
>   autotests/httpserver_p.cpp PRE-CREATION 
>   src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 
> 
> Diff: https://git.reviewboard.kde.org/r/127076/diff/
> 
> 
> Testing
> ---
> 
> Unit test + watching kio_http debug output in my current work on 
> keditbookmarks favicon support.
> 
> The missing Content-Type is due to another bug, but that's for the next RR.
> 
> 
> Thanks,
> 
> David Faure
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127076: kio_http: fix mimetype determination when URL ends with '/'.

[David Faure]

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

(Updated Feb. 14, 2016, 10:24 p.m.)


Review request for KDE Frameworks, Dawit Alemayehu and Andreas Hartmetz.


Changes
---

moved some code to the .cpp file as suggested by Milian + added const


Repository: kio


Description
---

When no Content-Type is sent by the server, we determine mimetype from
the URL and the contents. In the switch from KMimeType (which
took the fileName of the URL) and QMimeDatabase (which takes the full path),
we hit a difference: if the path ends with '/' then QMimeDatabase
assumes it's a directory, which isn't the case over HTTP.
So remove the trailing slash.

This commit introduces a test harness for kio_http: a basic HTTP server
running in a separate thread, which I wrote for KDSoap (LGPL).


Diffs (updated)
-

  autotests/CMakeLists.txt 107263820136c599df84c7beb06a29a1c52898ae 
  autotests/http_jobtest.cpp PRE-CREATION 
  autotests/httpserver_p.h PRE-CREATION 
  autotests/httpserver_p.cpp PRE-CREATION 
  src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 

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


Testing
---

Unit test + watching kio_http debug output in my current work on keditbookmarks 
favicon support.

The missing Content-Type is due to another bug, but that's for the next RR.


Thanks,

David Faure

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files

[Alex Richardson]

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



Looks good to me.

Possibly switch the if/else to have the short case at the top.


src/platformtheme/kdeplatformfiledialoghelper.cpp (line 202)


Maybe also pass `QUrl::StripTrailingSlash` here?


- Alex Richardson


On Feb. 14, 2016, 6:25 a.m., Kåre Särs wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126876/
> ---
> 
> (Updated Feb. 14, 2016, 6:25 a.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and David Faure.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> Qt does not know if the remote URL points to a file or directory. that is why 
> options()->initialDirectory() returns the full URL even if it is a file.
> 
> 
> This fix is a bit like Alex Richardson workaround in KIO 
> (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in 
> stead (I did not see his/Your KIO fix before now...)
> 
> I check the remote url in setDirectory() because setDirectoy() is called from 
> two places.
> 
> 
> Diffs
> -
> 
>   src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb 
> 
> Diff: https://git.reviewboard.kde.org/r/126876/diff/
> 
> 
> Testing
> ---
> 
> Kate now happily opens local and remote folders :)
> 
> 
> File Attachments
> 
> 
> firefox.desktop
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/01/25/60c21962-396e-468e-add9-e7112c49d7ba__firefox.desktop
> 
> 
> Thanks,
> 
> Kåre Särs
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 127077: kio_http: fix mimetype determination when URL ends with '/'.

[David Faure]

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

Review request for KDE Frameworks, Dawit Alemayehu, Andreas Hartmetz, and Rolf 
Eike Beer.


Repository: kio


Description
---

When no Content-Type is sent by the server, we determine mimetype from
the URL and the contents. In the switch from KMimeType (which
took the fileName of the URL) and QMimeDatabase (which takes the full path),
we hit a difference: if the path ends with '/' then QMimeDatabase
assumes it's a directory, which isn't the case over HTTP.
So remove the trailing slash.

This commit introduces a test harness for kio_http: a basic HTTP server
running in a separate thread, which I wrote for KDSoap (LGPL).

REVIEW: 127076

kio_http: read and discard body after a 404 with errorPage=false.

When getting a 404 error with some content, the job succeeds and returns content
(good for webbrowsers).
The metadata "errorPage" can be set to false so that the job fails instead
(useful for other cases, like favicon download, file copy etc.)
However the rest of the headers, as well as the body must still be read and
discarded, otherwise they clobber the next request (kio_http then starts
parsing in the middle of some headers and says "DO NOT WANT").

This is not a porting bug, I could reproduce it with kdelibs4 too.


Diffs
-

  autotests/CMakeLists.txt 107263820136c599df84c7beb06a29a1c52898ae 
  autotests/http_jobtest.cpp PRE-CREATION 
  autotests/httpserver_p.h PRE-CREATION 
  autotests/httpserver_p.cpp PRE-CREATION 
  src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 

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


Testing
---

Unittest.

Please review this patch carefully, I don't have much experience with this code 
in kio_http, and the potential for regressions in cases that I didn't test is 
likely high.


Thanks,

David Faure

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 127076: kio_http: fix mimetype determination when URL ends with '/'.

[David Faure]

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

Review request for KDE Frameworks, Dawit Alemayehu and Andreas Hartmetz.


Repository: kio


Description
---

When no Content-Type is sent by the server, we determine mimetype from
the URL and the contents. In the switch from KMimeType (which
took the fileName of the URL) and QMimeDatabase (which takes the full path),
we hit a difference: if the path ends with '/' then QMimeDatabase
assumes it's a directory, which isn't the case over HTTP.
So remove the trailing slash.

This commit introduces a test harness for kio_http: a basic HTTP server
running in a separate thread, which I wrote for KDSoap (LGPL).


Diffs
-

  autotests/CMakeLists.txt 107263820136c599df84c7beb06a29a1c52898ae 
  autotests/http_jobtest.cpp PRE-CREATION 
  autotests/httpserver_p.h PRE-CREATION 
  autotests/httpserver_p.cpp PRE-CREATION 
  src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 

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


Testing
---

Unit test + watching kio_http debug output in my current work on keditbookmarks 
favicon support.

The missing Content-Type is due to another bug, but that's for the next RR.


Thanks,

David Faure

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127077: kio_http: fix mimetype determination when URL ends with '/'.

[David Faure]

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

(Updated Feb. 14, 2016, 7:40 p.m.)


Review request for KDE Frameworks, Dawit Alemayehu, Andreas Hartmetz, and Rolf 
Eike Beer.


Changes
---

fix patch to have only one commit instead of two (wrong rbt usage)


Repository: kio


Description (updated)
---

kio_http: read and discard body after a 404 with errorPage=false.

When getting a 404 error with some content, the job succeeds and returns content
(good for webbrowsers).
The metadata "errorPage" can be set to false so that the job fails instead
(useful for other cases, like favicon download, file copy etc.)
However the rest of the headers, as well as the body must still be read and
discarded, otherwise they clobber the next request (kio_http then starts
parsing in the middle of some headers and says "DO NOT WANT").

This is not a porting bug, I could reproduce it with kdelibs4 too.


Diffs (updated)
-

  autotests/http_jobtest.cpp PRE-CREATION 
  src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 

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


Testing
---

Unittest.

Please review this patch carefully, I don't have much experience with this code 
in kio_http, and the potential for regressions in cases that I didn't test is 
likely high.


Thanks,

David Faure

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

building everything with a set of specific preprocessor token(s)/definition(s)?

[René J . V . Bertin]

Hi,

I'd like to investigate something Qt-related that would require me to build all 
frameworks with a specific set of preprocessor tokens defined. Rather than 
patching all toplevel CMake files I'm hoping for way to define those tokens in 
the cmake invocation, IOW on the cmake command line.
It seems CMake itself doesn't make that easy because of a lack incremental 
mechanisms, but maybe there's a hook in the ECM that I've overlooked?

Thanks,
René
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: building everything with a set of specific preprocessor token(s)/definition(s)?

[Boudhayan Gupta]

On 14 February 2016 at 19:31, René J.V.  wrote:
> Hi,
>
> I'd like to investigate something Qt-related that would require me to build 
> all frameworks with a specific set of preprocessor tokens defined. Rather 
> than patching all toplevel CMake files I'm hoping for way to define those 
> tokens in the cmake invocation, IOW on the cmake command line.
> It seems CMake itself doesn't make that easy because of a lack incremental 
> mechanisms, but maybe there's a hook in the ECM that I've overlooked?
>
> Thanks,
> René

CC="gcc -DWHATEVER_DEFINE" cmake ...

It really should be as simple as that.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: building everything with a set of specific preprocessor token(s)/definition(s)?

[David Faure]

On Sunday 14 February 2016 15:01:43 René J.V. Bertin wrote:
> Hi,
> 
> I'd like to investigate something Qt-related that would require me to build 
> all frameworks with a specific set of preprocessor tokens defined. Rather 
> than patching all toplevel CMake files I'm hoping for way to define those 
> tokens in the cmake invocation, IOW on the cmake command line.
> It seems CMake itself doesn't make that easy because of a lack incremental 
> mechanisms, but maybe there's a hook in the ECM that I've overlooked?

export CXXFLAGS="-DMYDEFINE=1" before running cmake should do the trick.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: building everything with a set of specific preprocessor token(s)/definition(s)?

[Boudhayan Gupta]

On 14 February 2016 at 22:00, René J.V.  wrote:
> On Sunday February 14 2016 15:20:14 David Faure wrote:
>
>> export CXXFLAGS="-DMYDEFINE=1" before running cmake should do the trick.
>
> That's what I'm using ATM, but cmake has a nasty habit of not taking the 
> user's full CXXFLAGS and/or adding its own stuff to the resulting 
> CMAKE_CXX_COMPILER_FLAGS_XXX, depending on what build type you're using.
>
> There's also an interaction with the build system I'm targeting; not really 
> relevant on here but it does make it less trivial to rely on settings passed 
> through the environment.

Hence my idea of using CC/CXX rather than the *FLAGS env-vars.

-- Boudhayan Gupta
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: building everything with a set of specific preprocessor token(s)/definition(s)?

[René J . V . Bertin]

On Sunday February 14 2016 15:20:14 David Faure wrote:

> export CXXFLAGS="-DMYDEFINE=1" before running cmake should do the trick.

That's what I'm using ATM, but cmake has a nasty habit of not taking the user's 
full CXXFLAGS and/or adding its own stuff to the resulting 
CMAKE_CXX_COMPILER_FLAGS_XXX, depending on what build type you're using.

There's also an interaction with the build system I'm targeting; not really 
relevant on here but it does make it less trivial to rely on settings passed 
through the environment.

R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126291: initial implementation of a platform plugin for OS X (WIP)

[René J . V . Bertin]

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

(Updated Feb. 14, 2016, 5:44 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

Updated for the current source and tested against FWs 5.19.0

It would be nice to get some feedback on this one, if not simply a simple "Ship 
It" ...


Repository: kwindowsystem


Description
---

KWindowSystem has been lacking a platform plugin for OS X. This RR presents a 
"backport" of the modified KDE4 KWindowSystem implementation that has been used 
in the MacPorts kdelibs4 port for the last 2 or 3 (or more) years.

I have made some initial steps to remove deprecated Carbon API calls, but this 
is clearly a work in progress.

Open questions include
- is there any justification to run an event handler (or Cocoa observer) to 
keep track of running applications, possibly even listing all their open 
windows?
- is there any use for the Qt event listener framework (cf. the NETEventFilter 
in the X11 plugin)? I haven't yet had time to try to figure out what this 
"could be good for", and am very open to suggestions in this departments.
- one application for such an event filter would be a listener that catches the 
opening and closing of all windows by the running process, and keeps track of 
their `WId`s. A new method could then be added (to `KWindowInfo`?) to 
distinguish `WId`s created by the running application from "foreign" ones 
(useful also on Wayland and MS Windows).

`KWindowSystem::setMainWindow` should become a front for payload provided by 
the plugins. I'll leave that to the regular/official maintainer(s) of this 
framework.

This code could probably do with *lots* of comments; I'll try to add them as 
questions about this or that bit of code arise.


Diffs (updated)
-

  src/kwindowsystem.cpp 407a67d 
  src/platforms/osx/CMakeLists.txt 4fc3347 
  src/platforms/osx/cocoa.json PRE-CREATION 
  src/platforms/osx/kkeyserver.cpp 3ddb921 
  src/platforms/osx/kwindowinfo.cpp e8555bb 
  src/platforms/osx/kwindowinfo.mm PRE-CREATION 
  src/platforms/osx/kwindowinfo_mac_p.h c8f307e 
  src/platforms/osx/kwindowinfo_p_cocoa.h PRE-CREATION 
  src/platforms/osx/kwindowsystem.cpp 1758829 
  src/platforms/osx/kwindowsystem_mac_p.h PRE-CREATION 
  src/platforms/osx/kwindowsystem_macobjc.mm PRE-CREATION 
  src/platforms/osx/kwindowsystem_p_cocoa.h PRE-CREATION 
  src/platforms/osx/plugin.h PRE-CREATION 
  src/platforms/osx/plugin.cpp PRE-CREATION 

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


Testing
---

On OS X 10.9.5 with Qt 5.5.1 and frameworks 5.16.0 .


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel