D6331: Make sure that the tsfiles target is generated

2017-06-21 Thread Burkhard Lück
lueck added a comment.


  locale fr or uk build now, but without the script files beeing installed.
  Looks like ./scripts/autogen.sh needs to be adapted as well

REPOSITORY
  R249 KI18n

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

To: apol, #frameworks, ltoscano, lueck


D6331: Make sure that the tsfiles target is generated

2017-06-21 Thread Aleix Pol Gonzalez
apol created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  I'm not sure how it's possible that pofiles is defined but tsfiles
  isn't, but this does fix the issue reported by Luigi:
  CMake Error at /usr/lib64/cmake/KF5I18n/KF5I18NMacros.cmake:129
  (add_dependencies):
  
Cannot add target-level dependencies to non-existent target "tsfiles".

TEST PLAN
  Built `svn+ssh://s...@svn.kde.org/home/kde/trunk/l10n-kf5/fr` by doing
  `./scripts/autogen.sh fr; mkdir build; cd build; cmake ../fr`

REPOSITORY
  R249 KI18n

BRANCH
  master

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

AFFECTED FILES
  cmake/KF5I18NMacros.cmake

To: apol, #frameworks, ltoscano, lueck


D6328: add unit test for isocpp

2017-06-21 Thread jonathan poelen
jpoelen created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  ioscpp-test

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

AFFECTED FILES
  autotests/folding/highlight.cpp.fold
  autotests/html/highlight.cpp.html
  autotests/input/highlight.cpp
  autotests/reference/highlight.cpp.ref

To: jpoelen
Cc: #frameworks


D6197: Add basic KAuth support to file ioslave

2017-06-21 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Looks good to me now, just minor issues.

INLINE COMMENTS

> file.cpp:1407-1408
> +{
> +int status = messageBox(WarningContinueCancel, warningMessage(warnId), 
> QStringLiteral("Warning!"),
> +QStringLiteral("Continue"), 
> QStringLiteral("Cancel"));
> +return status != 2;

Missing i18n() here

> file.cpp:1409
> +QStringLiteral("Continue"), 
> QStringLiteral("Cancel"));
> +return status != 2;
> +}

Use `ButtonCode::Cancel` rather than hardcoding 2

REPOSITORY
  R241 KIO

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

To: chinmoyr, elvisangelaccio, #frameworks, dfaure
Cc: eliasp, aacid


D6197: Add basic KAuth support to file ioslave

2017-06-21 Thread Chinmoy Ranjan Pradhan
chinmoyr updated this revision to Diff 15702.
chinmoyr marked 2 inline comments as done.
chinmoyr added a comment.


  Removed the ifdef. Moved the execWithElevatedPrivilege() method to 
file_unix.cpp .

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6197?vs=15621=15702

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

AFFECTED FILES
  autotests/kiotesthelper.h
  src/core/jobuidelegateextension.h
  src/ioslaves/file/CMakeLists.txt
  src/ioslaves/file/file.cpp
  src/ioslaves/file/file.h
  src/ioslaves/file/file_unix.cpp
  src/ioslaves/file/kauth/CMakeLists.txt
  src/ioslaves/file/kauth/file.actions
  src/ioslaves/file/kauth/helper.cpp
  src/ioslaves/file/kauth/helper.h
  src/widgets/jobuidelegate.cpp
  src/widgets/jobuidelegate.h

To: chinmoyr, elvisangelaccio, #frameworks, dfaure
Cc: eliasp, aacid


Re: Kirigami in Frameworks

2017-06-21 Thread Jonathan Riddell
On 21 June 2017 at 15:00, Marco Martin  wrote:
> As there were no replies for quite a while, i assume there are no
> particular objections.
>
> so, how to proceed? what needs to be doe to do the actual move?

Does it comply with the policies (as much as they are relevant for QML)?
https://community.kde.org/Frameworks/Policies

Get David Faure to give his approval then see what the reponse to my
"who is authorised to move repos around?" thead is.
https://marc.info/?l=kde-core-devel=149806172721190=2

Jonathan


D6317: fix crash on Windows when deleting an instance of QtMultimediaExtractor

2017-06-21 Thread Matthieu Gallien
mgallien created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  one should delete the QMediaPlayer in the owner thread to avoid crash

TEST PLAN
  Tested with visual studio 2015 build on Windows

REPOSITORY
  R286 KFileMetaData

BRANCH
  master

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

AFFECTED FILES
  src/extractors/qtmultimediaextractor.cpp

To: mgallien, kde-frameworks-devel
Cc: #frameworks


D6249: FindQHelpGenerator: avoid picking up Qt4 version

2017-06-21 Thread Friedrich W. H. Kossebau
kossebau added a comment.


  @palimaka You have KDE push rights, correct? Will you have time this week to 
push this, or do you want someone/me to do that for you?
  Would be good to have this in as soon as possible, given tagging release is 
<2 weeks away :)

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

To: palimaka, #frameworks, kossebau, kfunk
Cc: alexeymin, asturmlechner, #build_system


D6313: WIP: Support device pixel ratio in icon loader and engine

2017-06-21 Thread Ken Vermette
kvermette added a comment.


  In https://phabricator.kde.org/D6313#118250, @davidedmundson wrote:
  
  > For SVG icons this is fine.
  >
  > For pixmap icons this is only part of the needed changes.
  >
  > We don't want to load the 16px image and then resize it, I think that's 
what this would do? That would be an unacceptable regression.
  >
  > We would need a folder containing the 16px icons at 2x. This is now part of 
the FD.O icon spec [1].  But that means updating all of our icon theme parsing 
and a much much bigger patch. 
  >  Or we could just special case SVGs here.
  >
  > 
[1]https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html
  
  
  Behaviorally speaking there's justification for ensuring SVGs are treated the 
same as PNGs in this case. Looking at the code we aren't shimming the SVGs 
specifically (unless I'm missing something), but I just wanted to chime in with 
this and nip special treatment for SVGs in the bud. In maintaining the Aether 
icon theme I would create links to the higher resolution icons, so you'd see 
something like a "16x16x2" folder point to my "32x32" 'native' folder, and a 
"16x16x3"->"48x48" folder, "32x32x2"->"64x64", etc. Just because we *can* scale 
SVG icons doesn't mean that behavior should be assumed, should the icon set 
have higher fidelity icons ready for HiDPI.

REPOSITORY
  R302 KIconThemes

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

To: broulik, kde-frameworks-devel, #plasma, #vdg
Cc: kvermette, cfeck, davidedmundson, plasma-devel, #frameworks, ZrenBot, 
spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart, lukas


D6309: KIconThemes: some additional details about themes & icons on Mac & MSWin

2017-06-21 Thread René J . V . Bertin
rjvbb set the repository for this revision to R302 KIconThemes.

REPOSITORY
  R302 KIconThemes

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

To: rjvbb, #frameworks
Cc: tfry, kde-mac, #frameworks


D6309: KIconThemes: some additional details about themes & icons on Mac & MSWin

2017-06-21 Thread René J . V . Bertin
rjvbb updated this revision to Diff 15694.
rjvbb added a comment.


  In fact any kind of plugin can do this that is loaded sufficiently early, or 
even an initialisation procedure in an additional library you link with (but 
that is maybe TMI and somewhat self-evident). I don't mind propaganda for my 
integration plugin but maybe here's not the most appropriate place ;)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6309?vs=15688=15694

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

AFFECTED FILES
  README.md

To: rjvbb, #frameworks
Cc: tfry, kde-mac, #frameworks


Re: Kirigami in Frameworks

2017-06-21 Thread Marco Martin
As there were no replies for quite a while, i assume there are no
particular objections.

so, how to proceed? what needs to be doe to do the actual move?

On Mon, Jun 5, 2017 at 2:42 PM, Marco Martin  wrote:
> Hi all,
> The Kirigami component set always was targeted to be eventually released as a
> framework, ideally tier 1. since a framework must depend at most from 2 Qt
> releases before the current one, it couldn't be released there yet.
> Now that Qt 5.9 is released, i would like to propose to move Kirigami in
> frameworks, to be relased in the main release cycle, and stop standalone
> releases from extragear.
>
> It strictly depends just from Qt stuff, so should be tier 1 (at runtime it can
> use optional styles that use features from Plasma, tough not having plasma
> installed doesn't touch its functionality in any part, if this ends up being a
> problem, i can move that style into plasma-integration)
>
> --
> Marco Martin


D6313: WIP: Support device pixel ratio in icon loader and engine

2017-06-21 Thread Christoph Feck
cfeck added inline comments.

INLINE COMMENTS

> cfeck wrote in kiconloader.cpp:863
> Please use some rounding here. Scaling factors such as 1.4 cannot be 
> represented exactly.
> 
> Either add some formatting specifiers, e.g. for three decimal places, or use 
> qRound(scale * 1000).

Just checked that the default precision is 6 digits, so maybe not that 
important.

REPOSITORY
  R302 KIconThemes

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

To: broulik, kde-frameworks-devel, #plasma, #vdg
Cc: cfeck, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, 
lukas


D6313: WIP: Support device pixel ratio in icon loader and engine

2017-06-21 Thread Christoph Feck
cfeck added inline comments.

INLINE COMMENTS

> kiconloader.cpp:1299
>  
> -if (d->findCachedPixmapWithPath(key, pix, path)) {
> +if (d->findCachedPixmapWithPath(key, pix, path)) {// skip cache
>  if (path_store) {

Is this a left-over change from disabling this check?

REPOSITORY
  R302 KIconThemes

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

To: broulik, kde-frameworks-devel, #plasma, #vdg
Cc: cfeck, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, 
lukas


D6313: WIP: Support device pixel ratio in icon loader and engine

2017-06-21 Thread Christoph Feck
cfeck added inline comments.

INLINE COMMENTS

> kiconloader.cpp:863
> +   % QLatin1Char('@')
> +   % QString::number(scale)
> % QLatin1Char('_')

Please use some rounding here. Scaling factors such as 1.4 cannot be 
represented exactly.

Either add some formatting specifiers, e.g. for three decimal places, or use 
qRound(scale * 1000).

> kiconloader.h:247
>  
> +// FIXME docs
> +QPixmap loadIcon(const QString , KIconLoader::Group group, qreal 
> scale, int size = 0,

FIXME

REPOSITORY
  R302 KIconThemes

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

To: broulik, kde-frameworks-devel, #plasma, #vdg
Cc: cfeck, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, 
lukas


D6309: KIconThemes: some additional details about themes & icons on Mac & MSWin

2017-06-21 Thread Thomas Friedrichsmeier
tfry added a comment.


  Much clearer, now, IMO.
  
  Your osx-integration plugin is what I was thinking of, when writing about 
"patched libs". See, that's how confused I am, I just can't seem to remember 
any of this...
  
  How about adding that detail, too, e.g.:
  
... provided that the location is registered by calling 
QIcon::setThemeSearchPath(), or a suitable platform theme "integration" plugin 
is installed and loaded.

REPOSITORY
  R302 KIconThemes

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

To: rjvbb, #frameworks
Cc: tfry, kde-mac, #frameworks


KDE CI: Frameworks syntax-highlighting kf5-qt5 WindowsQt5.7 - Build # 22 - Fixed!

2017-06-21 Thread no-reply
BUILD SUCCESS
 Build URL
https://build-sandbox.kde.org/job/Frameworks%20syntax-highlighting%20kf5-qt5%20WindowsQt5.7/22/
 Project:
Frameworks syntax-highlighting kf5-qt5 WindowsQt5.7
 Date of build:
Wed, 21 Jun 2017 13:44:23 +
 Build duration:
3 min 7 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 8 test(s), Skipped: 0 test(s), Total: 8 test(s)

build.log
Description: Binary data


D6313: WIP: Support device pixel ratio in icon loader and engine

2017-06-21 Thread David Edmundson
davidedmundson added a comment.


  For SVG icons this is fine.
  
  For pixmap icons this is only part of the needed changes.
  
  We don't want to load the 16px image and then resize it, I think that's what 
this would do? That would be an unacceptable regression.
  
  We would need a folder containing the 16px icons at 2x. This is now part of 
the FD.O icon spec [1].  But that means updating all of our icon theme parsing 
and a much much bigger patch. 
  Or we could just special case SVGs here.
  
  
[1]https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html

INLINE COMMENTS

> kiconloader.cpp:1294
>  QPixmap pix;
> +pix.setDevicePixelRatio(scale);
> +

I don't think you need this?

REPOSITORY
  R302 KIconThemes

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

To: broulik, kde-frameworks-devel, #plasma, #vdg
Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas


D6220: CMake: Fix CMP0058 warning when using Ninja

2017-06-21 Thread Kevin Funk
kfunk added reviewers: vkrause, dhaumann.

REPOSITORY
  R216 Syntax Highlighting

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

To: kfunk, vkrause, dhaumann
Cc: #frameworks


D6313: WIP: Support device pixel ratio in icon loader and engine

2017-06-21 Thread Kai Uwe Broulik
broulik created this revision.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  Since Qt 5.9 there's a ScaledPixmapHook in QIconEngine which is called when 
device pixel ratio is > 1 and it wants a scaled pixmap. In contrast to regular 
pixmap this also knows the scale factor.
  
  This way, when a 32px icon is requested, we can now tell whether we want a 
16px scaled 2x or if it's a legitimate 32px request. It ensures that we keep 
symbolic small icons even on high-dpi screens where we would otherwise load the 
colorful and hard-to-see icons at the given physical size they ends up at.

TEST PLAN
  Ran `QT_SCREEN_SCALE_FACTORS=3 kate`:
  Before:
  F3789197: Screenshot_20170621_151044.png 

  After:
  F3789196: Screenshot_20170621_144649.png 

  
  Before:
  F3789201: Screenshot_20170621_151140.png 

  After:
  F3789199: Screenshot_20170621_151118.png 


REPOSITORY
  R302 KIconThemes

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

AFFECTED FILES
  src/kiconengine.cpp
  src/kiconengine.h
  src/kiconloader.cpp
  src/kiconloader.h

To: broulik, kde-frameworks-devel, #plasma, #vdg
Cc: plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas


D6309: KIconThemes: some additional details about themes & icons on Mac & MSWin

2017-06-21 Thread René J . V . Bertin
rjvbb set the repository for this revision to R302 KIconThemes.

REPOSITORY
  R302 KIconThemes

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

To: rjvbb, #frameworks
Cc: tfry, kde-mac, #frameworks


D6309: KIconThemes: some additional details about themes & icons on Mac & MSWin

2017-06-21 Thread René J . V . Bertin
rjvbb updated this revision to Diff 15688.
rjvbb added a comment.


  To be honest I didn't even think of patched libs, certainly not Qt itself. 
What I *can* think of is a platform theme plugin like the one from 
plasma-integration, which can serve a comparable purpose (including making the 
the call to setThemeSearchPath()) even with fully autonomous, standalone 
applications (cf. my osx-integration project).
  
  I've followed your rewrite, but elaborated on it, and removed the somewhat 
confusing wording that combines the concept of standalone applications with a 
(shared) icon theme in a (system) location. While technically possible such 
applications aren't truly standalone.
  
  Side remark: AFAIK only breeze-icons provides the BINARY_ICONS_RESOURCE 
option, at least in v5.35.0 . It would probably make sense to move the 
generate_binary_resource() macro from breeze-icons to the ECM, and make a 
beginning with the implementation of a few other convenience features for 
building projects for standalone deployment (a predefined macro to indicate the 
icon theme rcc to embed, but also something like the APPLE_STANDALONE_APPBUNDLE 
option I've used elsewhere). Mostly beyond the current scope though such 
changes would affect the text we're discussing here.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6309?vs=15674=15688

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

AFFECTED FILES
  README.md

To: rjvbb, #frameworks
Cc: tfry, kde-mac, #frameworks


D6215: Make sure size is final after showEvent

2017-06-21 Thread Marco Martin
This revision was automatically updated to reflect the committed changes.
Closed by commit R242:eab4aa9909a6: Make sure size is final after showEvent 
(authored by mart).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6215?vs=15579=15682

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

AFFECTED FILES
  src/declarativeimports/core/tooltip.cpp
  src/plasmaquick/dialog.cpp

To: mart, #plasma, davidedmundson
Cc: sebas, hein, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart, lukas


D6215: Make sure size is final after showEvent

2017-06-21 Thread Marco Martin
mart edited the summary of this revision.
mart edited the test plan for this revision.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

To: mart, #plasma, davidedmundson
Cc: sebas, hein, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart, lukas


D6215: Make sure size is final after showEvent

2017-06-21 Thread David Edmundson
davidedmundson accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

To: mart, #plasma, davidedmundson
Cc: sebas, hein, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart, lukas


D6215: Make sure size is final after showEvent

2017-06-21 Thread Marco Martin
mart added a comment.


  so, on the 5 points:
  
  1. yes, is necessary, resizing windows in their show event is definitely not 
enough, causes events to arrive to reset to the old geometry in race with the 
setgeometry done there, if it's the qpa, if it's qwindow, if it's the 
windowmanager doing that i have no idea, i have spent a week looking into that 
and really don't want to dive more in that spaghetti, more than accepting "qt 
wants sizes set before the showevent, move is fine" as a fact and adapting to it
  2. no it's not doing that anymore, all it changes is having 
syncToMainItemSize/updateLayoutParameters actually work when they are called 
with the window not visible, exactly because of 1)
  3. yeah, perhaps it should do it in a separate commit, and should be done 
everywhere a plasmashellsurface is used, or you will have wrong positions after 
hide/show that don't cause a moveevent
  4. as i explained in https://phabricator.kde.org/D6216, not strictly 
necessary, but avoids many bindings and resizes that happen when the window 
gets shown and hidden, that shouldn't be seen by the user anyways but useless 
never the less
  5. is done for a specific scenario (that is seen in the dbusnotificationtest 
manual test in knotifications) A notification is visible, and its content is 
replaced while still visible, so the actions gets removed, the window gets 
resized, all notifications gets rearranged, then actions populated again, 
window resized again, all notifications resized again. it is not strictly a 
bug, the whole procedure looks ugly and glitchy, especially if the number of 
actions after repopulating is the same, no resize or reposition of 
notifications should happen at all.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: mart, #plasma, davidedmundson
Cc: sebas, hein, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart, lukas


D6277: Emit errors when keditbookmarks is missing

2017-06-21 Thread Valeriy Malov
valeriymalov updated this revision to Diff 15681.
valeriymalov added a comment.


  Toned down error from critical to warning, removed const cast

REPOSITORY
  R294 KBookmarks

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6277?vs=15608=15681

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

AFFECTED FILES
  src/kbookmarkmanager.cpp
  src/kbookmarkmanager.h

To: valeriymalov, #frameworks
Cc: aacid, ltoscano


D6277: Emit errors when keditbookmarks is missing

2017-06-21 Thread Valeriy Malov
valeriymalov marked 2 inline comments as done.

REPOSITORY
  R294 KBookmarks

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

To: valeriymalov, #frameworks
Cc: aacid, ltoscano


D6309: KIconThemes: some additional details about themes & icons on Mac & MSWin

2017-06-21 Thread Thomas Friedrichsmeier
tfry added a comment.


  It's difficult to explain a confusing situation, clearly, but I don't think 
you succeeded. AFAIU, there are //three// distinct approaches:
  a) embedded icons
  b) QIcon::setThemeSearchPaths()
  c) patched libs
  
  From the current wording I'm not sure, whether you do or do not want to hint 
at c) at all. For a) and b), I suggest the following wording:
  
On other platforms such as Windows and Mac OS, icon themes are not 
regularly part of the system,
though they are available through package management systems (like 
MacPorts, Fink and
Cygwin).

The deployment strategy for creating standalone applications on those 
operations systems is to either a) make sure the required icon theme is 
installed in a suitable (system) location, and use call 
QIcon::setThemeSearchPaths(), or b) to embed icons as a binary resource as 
follows:

REPOSITORY
  R302 KIconThemes

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

To: rjvbb, #frameworks
Cc: tfry, kde-mac, #frameworks


D6215: Make sure size is final after showEvent

2017-06-21 Thread David Edmundson
davidedmundson added a comment.


  > no, it just means that who calls show() or the wrong setVisible() would 
just get the previous behavior of mainItem being shown only at showevent,
  
  Ok, great
  
  
  
  It's somewhat confusing as you have multiple completely independent attempts 
to solve the same problem.
  
  We have a patch now consists of:
  
  1. an early mainItem->setVisible()
  2. always updating the platform window size regardless of whether it's visible
  3. some other wayland changes (which aren't in your commit message)
  
  And we have another patch that:
  
  4. removes use of item::visible in working out window size
  5. inhibits resizing whilst we re-populate actions whilst invisible
  
  I'm after some explanation of what the problem(s) each one of those is 
solving.
  
  If 1 works, I don't see what 2 accomplishes, you're setting the platform 
window size earlier, but to something that we know is wrong.
  
  Also if 1 works, we don't need 4 or 5? Unless it's because notification does 
is doing the positioning before the show event?  At which point we could just 
fix that more normally.

INLINE COMMENTS

> dialog.cpp:1179
>  d->updateVisibility(true);
> +d->updateTheme();
>  }

what's this about?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: mart, #plasma, davidedmundson
Cc: sebas, hein, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart, lukas


D6215: Make sure size is final after showEvent

2017-06-21 Thread Marco Martin
mart added a comment.


  ping?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: mart, #plasma, davidedmundson
Cc: sebas, hein, davidedmundson, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, apol, mart, lukas


D6309: KIconThemes: some additional details about themes & icons on Mac & MSWin

2017-06-21 Thread René J . V . Bertin
rjvbb created this revision.
rjvbb added a project: Frameworks.

REVISION SUMMARY
  This clarifies a few details about icons and icon themes on Mac and MS 
Windows, calling attention to the fact that icon themes can be available as 
usual and that embedded icon resources are thus not the only way applications 
can have access to such themes.
  
  While not directly related I think this would also be a good place to draw 
attention to the fact that the usual
  
app.setWindowIcon(QIcon::fromTheme(appName)));
  
  can have the opposite effect on those systems and should thus be avoided or 
use the QIcon::fromTheme() fallback argument
  
app.setWindowIcon(QIcon::fromTheme(appName, app.windowIcon()));

REPOSITORY
  R302 KIconThemes

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

AFFECTED FILES
  README.md

To: rjvbb, #frameworks
Cc: kde-mac, #frameworks