D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng added a comment.


  In D29668#669069 , @dfaure wrote:
  
  > What are the values of Context and Type?
  >
  > "Legacy" and "UI" ?
  >
  > I can't see anything in index.theme 
https://ftp.gnome.org/pub/GNOME/sources/adwaita-icon-theme/3.36/
  
  
  Yes, legacy and ui.
  
  [8x8/legacy]
  Context=Legacy
  Size=8
  Type=Fixed

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng added a comment.


  You can try to install one from your distro to check the actual content. 
index.theme is generated during the "configure & make".

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29789: Make text always align with font base line

[Xuetian Weng]

xuetianweng added a comment.


  In D29789#672335 , @dhaumann wrote:
  
  > I like this patch. In fact, I observed over the past years again and again 
that sometimes, especially if chinese or similar letters are included, the 
baseline is wrong in Kate, leading to much more overpainting that needed.
  >  If this patch fixes this, then I'm all for it. Or let's put it like this: 
The current implementation is wrong, this patch looks less wrong than the 
current state :-)
  >
  > +1
  
  
  Actually if this patch is accepted, we may then want to add a configurable 
multiplier on line height  instead of enable variable lineheight support to 
resolve the character clip issue.
  Background painting to fill the gap to cover extra gap between line is 
probably easier and safer to implement.
  
  Equal line height would look better anyway.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, rjvbb, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D29789: Make text always align with font base line

[Xuetian Weng]

xuetianweng added a comment.


  This is my another try as an alternative solution to D25339 
. Actually this works surprisingly good 
IMHO, at least for CJK users for most cases.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, rjvbb, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D29789: Make text always align with font base line

[Xuetian Weng]

xuetianweng edited the summary of this revision.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, rjvbb, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D29789: Make text always align with font base line

[Xuetian Weng]

xuetianweng created this revision.
xuetianweng added reviewers: rjvbb, dhaumann, cullmann.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
xuetianweng requested review of this revision.

REVISION SUMMARY
  When font uses other fallback font, the base line may change to make all
  text on the same baseline. But when we render the text, we should always
  follow the baseline from FontMetrics to avoid text baseline jump up and
  down depending on the text in the line.

TEST PLAN
  Text manually with some mixed text.

REPOSITORY
  R39 KTextEditor

BRANCH
  master

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

AFFECTED FILES
  src/render/katerenderer.cpp
  src/render/katerenderer.h

To: xuetianweng, rjvbb, dhaumann, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: update lineHeight if boundingRect indicates a larger value.

[Xuetian Weng]

xuetianweng added a comment.


  Actually I was trying to use this approach in the patch because I was afraid 
that variable line height may need to estimate the whole document height to 
make scroll work correctly.
  
  But during this patching experience I realized that the ktexteditor first 
line is always a whole line. So the renderer only need to care about the 
current page instead of whole document, which makes variable line height might 
not as hard as thought.
  
  I won't have time recently to look into the variable line height though.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: ahmadsamir, brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng edited the test plan for this revision.

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng added reviewers: dfaure, apol.

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng edited the test plan for this revision.

REPOSITORY
  R302 KIconThemes

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

To: xuetianweng, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29668: Do not reject icon theme dir with invalid context/type.

[Xuetian Weng]

xuetianweng created this revision.
xuetianweng added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
xuetianweng requested review of this revision.

REVISION SUMMARY
  For example, gnome's default theme adwaita has their own context type like
  Legacy/UI. This check prevents the icon with such lines to be rendered for Qt 
app
  running under KDE. Even if it would result in some unexpected result, it's 
still
  better than not rendering the icon.

TEST PLAN
  Test with gnome default icon theme.

REPOSITORY
  R302 KIconThemes

BRANCH
  master

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

AFFECTED FILES
  src/kicontheme.cpp

To: xuetianweng, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D25339: update lineHeight if boundingRect indicates a larger value.

[Xuetian Weng]

xuetianweng marked an inline comment as done.
xuetianweng added a comment.


  In D25339#665432 , @rjvbb wrote:
  
  > With "we've ever seen" you do mean that lineheight only changes when a line 
that requires it scrolls into view?
  >
  > >   Though line height won't shrink during the edit phase, it will back to 
the actual value upon save.
  >
  > Have you tried to reset the max. lineheight on each redraw (I presume the 
scrollbars could give you a signal that a view scroll/jump was initiated, if 
need be). 
  >  Something tells me that it'd be nicer if lineheight always is as small as 
possible. Imagine you're using a smallish window to edit a document that just 
has some of these "offending", much higher characters at the bottom. If it gets 
into view only once, lineheight increases and it's as if you lose a lot of 
screen estate until you save the document. Now I mustn't be the only one who 
often doesn't save for a while, esp. when doing things like moving blocks of 
text around, and it's exactly in that kind of scenario where having to save to 
get a more space-efficient rendering back can be very annoying.
  >
  > As long as you can determine the max. lineheight required for the view 
that's about to be drawn before the view is actually drawn there should be no 
glitches. You'd just see a jump in lineheight and there would probably be an 
interesting problem to tackle with edge cases where the higher glyphs fall 
outside the view area because of the increased lineheight, but that's something 
your current implementation cannot avoid completely either. As to the changing 
lineheight: I think users will understand why it happens and get used to it. 
It's comparable to what you see in graphics programmes that show cursor 
co-ordinates next to the cursor; that indicator has to jump when it would get 
"pushed out" of the view, and that doesn't even feel weird.
  >
  > I presume your new approach would work not just for CJK characters, but 
also for anything else that changes the lineheight. That's and advantage but 
could also lead to regressions for some (who never use CJK characters or who, 
like me, wouldn't care how they display because they can't read them anyway). 
Emoticons come to mind; here too I don't really care if they get cut off. Scrap 
that, I *really* don't care if they are...
  
  
  To be honest,  I didn't see issue about color emoji (until select them). My 
"fill" gap code seems can't make color emoji bitmap transparent The fill 
gap code is indeed a hack.
  
  Probably when I get time I might try to make it able to render view with 
different height...
  
  As for higher line, it might not as bad as you thought as it actually might 
improve readability for many people.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D25339: update lineHeight if boundingRect indicates a larger value.

[Xuetian Weng]

xuetianweng retitled this revision from "KateRenderer: Use representitive 
character in CJK to estimate the fontHeight." to "update lineHeight if 
boundingRect indicates a larger value.".
xuetianweng edited the summary of this revision.
xuetianweng edited the test plan for this revision.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  I'm not sure if this is the right way to do it or it might cause any glitch,  
but here we go. Upon line rendering, update the maximum height we've ever seen.
  
  Though line height won't shrink during the edit phase, it will back to the 
actual value upon save.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 82170.
xuetianweng added a comment.


  Use actual line height instead of representitive character.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80607&id=82170

BRANCH
  arcpatch-D25339

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

AFFECTED FILES
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/view/kateview.h

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  In D25339#663915 , @fakefred wrote:
  
  > I second the as-an-option proposal. Hey, why not automatically increase the 
line height when CJK characters are detected?
  
  
  So the thing is we need to maintain a such height depending on the document, 
not so sure if that is even possible (or whether it could be costly) or you 
could give me some hint on that?
  Actually, I'd rather to calculate the height based on actual character in the 
document because during my test, mixing Arabic together to CJK and latin could 
still have problem (likely same for Cyrillic).

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D29396: Suppress find_package_handle_standard_args package name mismatch warning.

[Xuetian Weng]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:8d181637a033: Suppress find_package_handle_standard_args 
package name mismatch warning. (authored by xuetianweng).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29396?vs=81832&id=81908

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

AFFECTED FILES
  modules/ECMFindModuleHelpers.cmake

To: xuetianweng, #frameworks, #build_system, apol
Cc: apol, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns

D29396: Suppress find_package_handle_standard_args package name mismatch warning.

[Xuetian Weng]

xuetianweng created this revision.
xuetianweng added reviewers: Frameworks, Build System.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
xuetianweng requested review of this revision.

REVISION SUMMARY
  cmake introduced a new find_package mismatch check in 3.17, but also allows
  user to suppress the warning by setting a variable (Or parameter, but 
  require new cmake 3.17). Same technique is also used with in cmake,
  e.g. FindGTK2.cmake.

TEST PLAN
  Test with FindXCB.cmake

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  modules/ECMFindModuleHelpers.cmake

To: xuetianweng, #frameworks, #build_system
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80607.
xuetianweng added a comment.


  Fix the blank text issue by alway setLineWidth.
  
  For QTextLine, the layout information is not filled until certain function is 
called.
  If dynamic wrap is not enabled, the line width is -1 so at the point we do 
the calculation,
  we will have no size information of the text. Use INT_MAX to set the 
lineWidth as workaround.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80606&id=80607

BRANCH
  arcpatch-D25339

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80606.
xuetianweng added a comment.


  Clean up the code a little bit and adding comments. Also fix a small bug if 
m_fontHeight has 
  big difference with m_fontMetrics's height.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80444&id=80606

BRANCH
  arcpatch-D25339

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  In D25339#651701 , @cullmann wrote:
  
  > Hmm, after applying this patch, for me, no text is visible at all.
  >  By selecting a bit stuff, one at least sees an outline (CMakeLists.txt of 
ktexteditor toplevel dir).
  >
  > F8246692: kwrite_no_text.png 
  
  
  Are you sure it is the latest patch? I know some old version in this patch 
might have this effect.
  What's your color & font in kwrite setup BTW?

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  In D25339#651701 , @cullmann wrote:
  
  > Hmm, after applying this patch, for me, no text is visible at all.
  >  By selecting a bit stuff, one at least sees an outline (CMakeLists.txt of 
ktexteditor toplevel dir).
  >
  > F8246692: kwrite_no_text.png 
  
  
  let me test with some different colorscheme..

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80444.
xuetianweng added a comment.


  Fix the handling when layout formats has background.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80438&id=80444

BRANCH
  arcpatch-D25339

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  F8244684: Screenshot_20200417_222110.png 

  
  Add a screenshot to demostrate the change.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  In D25339#563937 , @cullmann wrote:
  
  > Actually, I could live with:
  >
  > 1. All lines are a bit higher, for me that makes reading even easier. But 
the rendering shall have no glitches.
  > 2. Some lines have different heights. But I assume this is hard to 
implement at the moment.
  
  
  After some experiment and reading the Qt code, seems there is no easy way to 
extend the line height..
  E.g. QPlainTextEdit will show the text with different line height, so I 
believe I couldn't do it with in Qt.
  
  So I tried to use a small trick to fill the gap.
  If there is some gap need to be filled, then the code tried to draw some 
transparent text so the background will be extended to fill the gap. Then draw 
the real text.
  
  The solution is hacky though.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80438.
xuetianweng added a comment.


  handle the range with multiple lines.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80433&id=80438

BRANCH
  arcpatch-D25339

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80433.
xuetianweng added a comment.


  Try to fill the gap if we increase the line height with representitive char.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=69848&id=80433

BRANCH
  arcpatch-D25339

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  Any idea about how konsole derive
  
  In D25339#563656 , @cullmann wrote:
  
  > The issue with this approach is, that you get "too high" lines for the case 
that none of this characters ever appear.
  >  Beside that, unfortunately, you get rendering artifacts in addition, as Qt 
will not paint e.g. selection high enough for lines without such chars.
  >
  > See e.g. attached screenshot.F7768081: rendering_issue.png 

  
  
  Then what is the ideal solution to you for this case?
  
  Having different font height for every line? Or keep this "slightly higher 
line" by do vertical center align  and fix the artifact?
  
  At least the current state doesn't look right to me and I think it need to be 
fixed.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added reviewers: cullmann, dhaumann.

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  F7766842: Screenshot_20191116_092819.png 

  
  Add screenshot to demonstrate the problem.
  
  And what I'd like to point is, for CJK users, it is uncommon for them to 
select a single font to cover all the characters, because such fonts are really 
rare. People usually select one latin only font and just let system 
(fontconfig) select the fallback for them.
  So this could be a common problem for them to see the "half character".

REPOSITORY
  R39 KTextEditor

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

To: xuetianweng, #ktexteditor
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 69848.
xuetianweng added a comment.


  Add comment about the actual characters.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=69847&id=69848

BRANCH
  master

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng created this revision.
xuetianweng added a reviewer: KTextEditor.
xuetianweng added projects: Kate, Frameworks.
xuetianweng requested review of this revision.

REVISION SUMMARY
  When a font has fallback, the fontHeight is broken can may cut the string
  in the middle in the line. Try to use some representive character from 
multiple
  languages to estimate the actual fontHeight.
  
  BUG: 404907

TEST PLAN
  Manually tested.

REPOSITORY
  R39 KTextEditor

BRANCH
  master

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

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D8800: Add a new function to measure the length by text.

[Xuetian Weng]

This revision was automatically updated to reflect the committed changes.
Closed by commit R244:6758d45cb596: Add a new function to measure the length by 
text. (authored by xuetianweng).

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8800?vs=22287&id=22288

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

AFFECTED FILES
  autotests/kstringhandlertest.cpp
  autotests/kstringhandlertest.h
  src/lib/text/kstringhandler.cpp
  src/lib/text/kstringhandler.h

To: xuetianweng, hein, apol, #frameworks
Cc: #frameworks

D8800: Add a new function to measure the length by text.

[Xuetian Weng]

xuetianweng added a reviewer: Frameworks.

REPOSITORY
  R244 KCoreAddons

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

To: xuetianweng, hein, apol, #frameworks
Cc: #frameworks

D8800: Add a new function to measure the length by text.

[Xuetian Weng]

xuetianweng created this revision.
xuetianweng added reviewers: hein, apol.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  CJK language contains more information per character. When people try to
  write code to cap the user input by length, ususally it makes more sense
  to loose limit for CJK strings. In real life, twitter now also employs
  similar algorithm to make tweet length 280 in latin or 140 in CJK characters.

TEST PLAN
  unit test.

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

AFFECTED FILES
  autotests/kstringhandlertest.cpp
  autotests/kstringhandlertest.h
  src/lib/text/kstringhandler.cpp
  src/lib/text/kstringhandler.h

To: xuetianweng, hein, apol
Cc: #frameworks

D5805: include directory should contains all directory from pkg-config

[Xuetian Weng]

xuetianweng abandoned this revision.
xuetianweng added a comment.


  Just noticed that FindPkgConfig has import_module now.. Personally I just 
port my code to use that.
  
  Anyone who more familiar with test cmake script may take over this.

REPOSITORY
  R240 Extra CMake Modules

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

To: xuetianweng, apol, aacid
Cc: #frameworks, #build_system

D8346: Remove blocking call in KStatusNotifierItem

[Xuetian Weng]

xuetianweng added inline comments.

INLINE COMMENTS

> kstatusnotifieritem.cpp:882
> +
> +QDBusPendingReply pendingReply = 
> statusNotifierWatcher->connection().call(call);
> +auto watcher = new QDBusPendingCallWatcher(pendingReply);

The template is not necessary here. Maybe auto or QDBusPendingCall ?

> kstatusnotifieritem.cpp:883
> +QDBusPendingReply pendingReply = 
> statusNotifierWatcher->connection().call(call);
> +auto watcher = new QDBusPendingCallWatcher(pendingReply);
> +QObject::connect(watcher, &QDBusPendingCallWatcher::finished, q, 
> [this, watcher]() {

I prefer to assign a parent to the watcher. Maybe "q"?

> kstatusnotifieritem.cpp:884
> +auto watcher = new QDBusPendingCallWatcher(pendingReply);
> +QObject::connect(watcher, &QDBusPendingCallWatcher::finished, q, 
> [this, watcher]() {
> +QDBusPendingReply reply = *watcher;

watcher can be obtained via the argument of this signal, no need to capture it.

REPOSITORY
  R289 KNotifications

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

To: davidedmundson
Cc: xuetianweng, #frameworks

D8346: Remove blocking call in KStatusNotifierItem

[Xuetian Weng]

xuetianweng added inline comments.

INLINE COMMENTS

> kstatusnotifieritem.cpp:879
> +} else {
> +QDBusMessage call = 
> QDBusMessage::createMethodCall(statusNotifierWatcher->service(), 
> statusNotifierWatcher->path(), 
> QStringLiteral("org.freedesktop.DBus.Properties."), QStringLiteral("Get"));
> +call.setArguments({statusNotifierWatcher->interface(), 
> QStringLiteral("IsStatusNotifierHostRegistered")});

Do you need the extra dot at the end of "org.freedesktop.DBus.Properties." ?

REPOSITORY
  R289 KNotifications

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

To: davidedmundson
Cc: xuetianweng, #frameworks

D7491: Fix invalid id in viewitem.

[Xuetian Weng]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:8d38df353422: Fix invalid id in viewitem. (authored by 
xuetianweng).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7491?vs=18618&id=19209

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

AFFECTED FILES
  src/desktoptheme/breeze/widgets/viewitem.svgz

To: xuetianweng, mart, #plasma, davidedmundson
Cc: #frameworks

D7491: Fix invalid id in viewitem.

[Xuetian Weng]

xuetianweng added reviewers: Plasma, davidedmundson.
xuetianweng added a comment.


  ping

REPOSITORY
  R242 Plasma Framework (Library)

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

To: xuetianweng, mart, #plasma, davidedmundson
Cc: #frameworks

D7491: Fix invalid id in viewitem.

[Xuetian Weng]

xuetianweng created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  It should be selected+hover instead of selected_hover.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

AFFECTED FILES
  src/desktoptheme/breeze/widgets/viewitem.svgz

To: xuetianweng, mart
Cc: #frameworks

D5631: Fix directory based search.

[Xuetian Weng]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:5f6bd28119c6: Fix directory based search. (authored by 
xuetianweng).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5631?vs=13905&id=17950

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

AFFECTED FILES
  src/lib/searchstore.cpp

To: xuetianweng, #frameworks, lbeltrame
Cc: elvisangelaccio, lbeltrame

D5805: include directory should contains all directory from pkg-config

[Xuetian Weng]

xuetianweng added reviewers: apol, aacid.

REPOSITORY
  R240 Extra CMake Modules

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

To: xuetianweng, apol, aacid
Cc: #frameworks, #build_system

D5805: include directory should contains all directory from pkg-config

[Xuetian Weng]

xuetianweng created this revision.
Restricted Application added projects: Frameworks, Build System.
Restricted Application added subscribers: Build System, Frameworks.

REVISION SUMMARY
  For certain pkg-config file that depends on other pkg-config, e.g. pango
  depends on glib2, glib's include directory would be omitted from the target,
  which essentially make it useless since the target will not compile with
  the only directory that contains the specified header.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  modules/ECMFindModuleHelpers.cmake

To: xuetianweng
Cc: #frameworks, #build_system

D5631: Fix directory based search.

[Xuetian Weng]

xuetianweng added a comment.


  I saw there's filesearchstoretest.cpp in autotests/integration, but 
CMakeLists.txt doesn't build it. I guess this test might not work yet.

REPOSITORY
  R293 Baloo

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

To: xuetianweng, #frameworks
Cc: lbeltrame

D5631: Fix directory based search.

[Xuetian Weng]

xuetianweng created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  canonicalPath() does not include the file name, but only the directory
  name. Should use canonicalFilePath() instead.

TEST PLAN
  Manually test with baloosearch

REPOSITORY
  R293 Baloo

BRANCH
  master

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

AFFECTED FILES
  src/lib/searchstore.cpp

To: xuetianweng, #frameworks

D5017: Fix ecm_generate_pkgconfig_file compatibility with new cmake

[Xuetian Weng]

This revision was automatically updated to reflect the committed changes.
Closed by commit R240:3980f36db018: Fix ecm_generate_pkgconfig_file 
compatibility with new cmake (authored by xuetianweng).

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5017?vs=12392&id=12401

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

AFFECTED FILES
  modules/ECMGeneratePkgConfigFile.cmake

To: xuetianweng, #frameworks, dfaure
Cc: #build_system

D5017: Fix ecm_generate_pkgconfig_file compatibility with new cmake

[Xuetian Weng]

xuetianweng created this revision.
Restricted Application added projects: Frameworks, Build System.
Restricted Application added a subscriber: Build System.

REVISION SUMMARY
  CMP0053 specifies that:
  
  - Expansion of ``@VAR@`` reference syntax defined by the ``configure_file()`` 
and ``string(CONFIGURE)`` commands is no longer performed in other contexts.
  
  replace it with the $ variable syntax.

TEST PLAN
  manual

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  modules/ECMGeneratePkgConfigFile.cmake

To: xuetianweng, #frameworks, dfaure
Cc: #build_system

D4769: Try to get the real port instead of always use DEFAULT_SFTP_PORT

[Xuetian Weng]

This revision was automatically updated to reflect the committed changes.
Closed by commit R320:40f1edc9a814: Try to get the real port instead of always 
use DEFAULT_SFTP_PORT (authored by xuetianweng).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D4769?vs=11784&id=12138#toc

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4769?vs=11784&id=12138

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

AFFECTED FILES
  sftp/kio_sftp.cpp

To: xuetianweng, #frameworks, apol, dfaure
Cc: dfaure

D4769: Try to get the real port instead of always use DEFAULT_SFTP_PORT

[Xuetian Weng]

xuetianweng marked 2 inline comments as done.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: xuetianweng, #frameworks, apol, dfaure
Cc: dfaure

Re: Review Request 127271: Disable session restore for kwalletd5

[Xuetian Weng]

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

(Updated April 6, 2016, 7:53 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Martin Klapetek.


Changes
---

Submitted with commit c3cd7b72b3b1c2a6f13225c0d9141992e1cc024d by Weng Xuetian 
to branch master.


Repository: kwallet


Description
---

I notice a kwalletd5 with "-session ." in its command line started on my 
desktop and kwallet-pam doesn't work.

Also kwalletd is dbus activated in other cases there's no point to let session 
manager to restore it.


Diffs
-

  src/runtime/kwalletd/main.cpp 740e670 

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


Testing
---

kwallet-pam back to work.


Thanks,

Xuetian Weng

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

Re: Review Request 127271: Disable session restore for kwalletd5

[Xuetian Weng]

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

(Updated March 11, 2016, 4:26 p.m.)


Review request for KDE Frameworks and Martin Klapetek.


Repository: kwallet


Description
---

I notice a kwalletd5 with "-session ." in its command line started on my 
desktop and kwallet-pam doesn't work.

Also kwalletd is dbus activated in other cases there's no point to let session 
manager to restore it.


Diffs (updated)
-

  src/runtime/kwalletd/main.cpp 740e670 

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


Testing
---

kwallet-pam back to work.


Thanks,

Xuetian Weng

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

Re: Review Request 127166: Fix xcb port of klauncher and clean up the code.

[Xuetian Weng]

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

(Updated March 8, 2016, 12:17 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, David Faure and Martin Gräßlin.


Changes
---

Submitted with commit d154b4c0d85c1616c512cabdf5bf9919a9a9191d by Weng Xuetian 
to branch master.


Repository: kinit


Description
---

The ported klauncher at least has two bugs:
1. IMHO, it should compare the cached display string with == instead of != 
(though != has been there since the old kdelibs, but it just doesn't look 
correct to me, what's the point of assign dpy = mCached_dpy when dpy_str != 
XDisplayString(dpy)? And what's the point use == in one place and != in other 
two places? ).
2. it might free the cached connection without setting mCached.conn back to 
nullptr, which could lead to double free or freeze when system logout.

This patch refactor the code a little bit with a helper function to update the 
cached connection instead of duplicate the same logic everywhere. 
getXCBConnection() will make sure the returned connection is error-free, reuse 
the cached connection if possible, and update the cached connection if needed.


Diffs
-

  src/klauncher/klauncher.h 31bfaaa 
  src/klauncher/klauncher.cpp 7ea9da9 

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


Testing
---

Compiles, startup notify works.


Thanks,

Xuetian Weng

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

Re: Review Request 127271: Disable session restore for kwalletd5

[Xuetian Weng]

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

(Updated March 6, 2016, 7:01 p.m.)


Review request for KDE Frameworks and Martin Klapetek.


Repository: kwallet


Description
---

I notice a kwalletd5 with "-session ." in its command line started on my 
desktop and kwallet-pam doesn't work.

Also kwalletd is dbus activated in other cases there's no point to let session 
manager to restore it.


Diffs (updated)
-

  src/runtime/kwalletd/main.cpp 740e670 

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


Testing
---

kwallet-pam back to work.


Thanks,

Xuetian Weng

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

Re: Review Request 127271: Disable session restore for kwalletd5

[Xuetian Weng]

> On March 5, 2016, 9:27 a.m., David Faure wrote:
> > Looks good and more portable than the
> > qunsetenv("SESSION_MANAGER");
> > which is used in many other places...
> > 
> > Not sure both connects are necessary though?
> 
> Xuetian Weng wrote:
> This is not to disable the whole session manager thing, but just disable 
> the restore. So session manager is still able to terminate kwalletd.
> 
> David Faure wrote:
> Ah. Do you mean that with this code, the process is terminated more 
> gracefully than with the qunsetenv solution which e.g. kiod or kded use (I 
> guess they simply die with an X error when Xorg disappears). Sounds like 
> another reason for this approach indeed, although it probably doesn't make 
> much difference from a user's perspective [other than portability of course].
> 
> Sounds like we should apply the same code in all these:
> 
> kded/src/kded.cpp:682:qunsetenv("SESSION_MANAGER");
> kdesu/src/ptyprocess.cpp:305:unsetenv("SESSION_MANAGER");
> kglobalaccel/src/runtime/main.cpp:56:qunsetenv( "SESSION_MANAGER" );
> kinit/src/klauncher/klauncher_main.cpp:153:
> qunsetenv("SESSION_MANAGER");
> kio/src/kiod/kiod_main.cpp:89:qunsetenv("SESSION_MANAGER"); 
> 
> This also makes me wonder what happens to non-GUI daemons, i.e. what will 
> terminate them when logging out... (all of the above are GUI enabled, but 
> e.g. kio_http_cache_cleaner is core-only).
> 
> Xuetian Weng wrote:
> Well, I doubt this method is useful for cases above. kiod/kded might not 
> want to be exit by session manager maybe? Because when session terminates, it 
> is still possible for application like kwrite/kate to use kio to write or 
> open something.
> 
> I think most of them (at least for klauncher/kded/kiod) are currently 
> doing the right thing.
> 
> kio_http_cache_cleaner should be managed by kio so I don't think we need 
> to worry about it. core-only application could possible leave some garbage 
> behind, for example: https://git.reviewboard.kde.org/r/125746/
> 
> David Faure wrote:
> Ah, but then it's the same for kwalletd if kwrite uses KIO to save to 
> an FTP directory, this might require a password, which would then require 
> kwalletd. I see no difference between kwalletd and kiod/kded in that respect.
> 
> But I don't even think that's a problem. Closing the session happens in 
> several steps (asking all apps to save state, then asking all apps to prompt 
> the user for saving open files, and only then closing all windows and 
> quitting the apps). So I don't think klauncher/kded/kiod/kwalletd being asked 
> to quit by the session manager would break anything, that's only the very 
> last step after all the saving is done.
> 
> kio_http_cache_cleaner is started by the first kio_http-using apps, but 
> quitting it is difficult: it's a "singleton" process, so kio_http can't just 
> make it quit on exit. It would need refcounting [difficult in case kio_http 
> crashes]  or indeed it could be QGuiApplication (not good for possible 
> core-only kio usage). Tricky.

Then you would need to use things like 
https://git.reviewboard.kde.org/r/127125/ . But I still think there could be 
some order issue. I get the idea that kwalletd could probably just use the 
unset QSESSION_MANAGER method.


- Xuetian


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


On March 3, 2016, 8:34 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127271/
> ---
> 
> (Updated March 3, 2016, 8:34 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Klapetek.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> I notice a kwalletd5 with "-session ." in its command line started on my 
> desktop and kwallet-pam doesn't work.
> 
> Also kwalletd is dbus activated in other cases there's no point to let 
> session manager to restore it.
> 
> 
> Diffs
> -
> 
>   src/runtime/kwalletd/main.cpp 740e670 
> 
> Diff: https://git.reviewboard.kde.org/r/127271/diff/
> 
> 
> Testing
> ---
> 
> kwallet-pam back to work.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 127271: Disable session restore for kwalletd5

[Xuetian Weng]

> On March 5, 2016, 9:27 a.m., David Faure wrote:
> > Looks good and more portable than the
> > qunsetenv("SESSION_MANAGER");
> > which is used in many other places...
> > 
> > Not sure both connects are necessary though?
> 
> Xuetian Weng wrote:
> This is not to disable the whole session manager thing, but just disable 
> the restore. So session manager is still able to terminate kwalletd.
> 
> David Faure wrote:
> Ah. Do you mean that with this code, the process is terminated more 
> gracefully than with the qunsetenv solution which e.g. kiod or kded use (I 
> guess they simply die with an X error when Xorg disappears). Sounds like 
> another reason for this approach indeed, although it probably doesn't make 
> much difference from a user's perspective [other than portability of course].
> 
> Sounds like we should apply the same code in all these:
> 
> kded/src/kded.cpp:682:qunsetenv("SESSION_MANAGER");
> kdesu/src/ptyprocess.cpp:305:unsetenv("SESSION_MANAGER");
> kglobalaccel/src/runtime/main.cpp:56:qunsetenv( "SESSION_MANAGER" );
> kinit/src/klauncher/klauncher_main.cpp:153:
> qunsetenv("SESSION_MANAGER");
> kio/src/kiod/kiod_main.cpp:89:qunsetenv("SESSION_MANAGER"); 
> 
> This also makes me wonder what happens to non-GUI daemons, i.e. what will 
> terminate them when logging out... (all of the above are GUI enabled, but 
> e.g. kio_http_cache_cleaner is core-only).

Well, I doubt this method is useful for cases above. kiod/kded might not want 
to be exit by session manager maybe? Because when session terminates, it is 
still possible for application like kwrite/kate to use kio to write or open 
something.

I think most of them (at least for klauncher/kded/kiod) are currently doing the 
right thing.

kio_http_cache_cleaner should be managed by kio so I don't think we need to 
worry about it. core-only application could possible leave some garbage behind, 
for example: https://git.reviewboard.kde.org/r/125746/


- Xuetian


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


On March 3, 2016, 8:34 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127271/
> ---
> 
> (Updated March 3, 2016, 8:34 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Klapetek.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> I notice a kwalletd5 with "-session ." in its command line started on my 
> desktop and kwallet-pam doesn't work.
> 
> Also kwalletd is dbus activated in other cases there's no point to let 
> session manager to restore it.
> 
> 
> Diffs
> -
> 
>   src/runtime/kwalletd/main.cpp 740e670 
> 
> Diff: https://git.reviewboard.kde.org/r/127271/diff/
> 
> 
> Testing
> ---
> 
> kwallet-pam back to work.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 127271: Disable session restore for kwalletd5

[Xuetian Weng]

> On March 5, 2016, 9:27 a.m., David Faure wrote:
> > Looks good and more portable than the
> > qunsetenv("SESSION_MANAGER");
> > which is used in many other places...
> > 
> > Not sure both connects are necessary though?

This is not to disable the whole session manager thing, but just disable the 
restore. So session manager is still able to terminate kwalletd.


- Xuetian


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


On March 3, 2016, 8:34 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127271/
> ---
> 
> (Updated March 3, 2016, 8:34 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Klapetek.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> I notice a kwalletd5 with "-session ." in its command line started on my 
> desktop and kwallet-pam doesn't work.
> 
> Also kwalletd is dbus activated in other cases there's no point to let 
> session manager to restore it.
> 
> 
> Diffs
> -
> 
>   src/runtime/kwalletd/main.cpp 740e670 
> 
> Diff: https://git.reviewboard.kde.org/r/127271/diff/
> 
> 
> Testing
> ---
> 
> kwallet-pam back to work.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 127261: Fix dead lock when program use kauth exits.

[Xuetian Weng]

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

(Updated March 5, 2016, 6:32 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Edmundson.


Changes
---

Submitted with commit 702475557d5d2f6680f01169b4e33881be9b35b7 by David Faure 
on behalf of Xuetian Weng to branch master.


Repository: kauth


Description
---

I'm on Qt 5.6 RC, and just notice a issue that kded5 never cleanly exits.

With gdb attached to kded5, it shows following backtrace:

#1  0x75c820eb in QWaitConditionPrivate::wait 
(time=18446744073709551615, this=0x74ac70) at thread/qwaitcondition_unix.cpp:136
#2  QWaitCondition::wait (this=this@entry=0x71c308, 
mutex=mutex@entry=0x71c300, time=time@entry=18446744073709551615) at 
thread/qwaitcondition_unix.cpp:208
#3  0x75c7b08b in QSemaphore::acquire 
(this=this@entry=0x7fffd840, n=n@entry=1) at thread/qsemaphore.cpp:137
#4  0x75e80fff in QMetaObject::activate 
(sender=sender@entry=0x976280, signalOffset=, 
local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x7fffd8d0)
at kernel/qobject.cpp:3698
#5  0x75e81687 in QMetaObject::activate 
(sender=sender@entry=0x976280, m=m@entry=0x76092760 
, local_signal_index=local_signal_index@entry=0, 
argv=argv@entry=0x7fffd8d0) at kernel/qobject.cpp:3595
#6  0x75e8172f in QObject::destroyed (this=this@entry=0x976280, 
_t1=_t1@entry=0x976280) at .moc/moc_qobject.cpp:213
#7  0x75e886e5 in QObject::~QObject (this=, 
__in_chrg=) at kernel/qobject.cpp:913
#8  0x7fffb39bbb09 in KAuth::DBusHelperProxy::~DBusHelperProxy 
(this=0x976280, __in_chrg=) at 
/chakra/core/kauth/src/kauth-5.19.0/src/backends/dbus/DBusHelperProxy.cpp:55
#9  0x75e49ff9 in QLibraryPrivate::unload (this=0x71dba0, 
flag=QLibraryPrivate::NoUnloadSys) at plugin/qlibrary.cpp:551
#10 0x75e4e521 in QLibraryStore::cleanup () at 
plugin/qlibrary.cpp:397
#11 0x778592ef in __cxa_finalize () from /usr/lib/libc.so.6
#12 0x75c52583 in __do_global_dtors_aux () from 
/usr/lib/libQt5Core.so.5
#13 0x7fffe230 in ?? ()
#14 0x77dea867 in _dl_fini () from /lib64/ld-linux-x86-64.so.2


Which indicates that kauth dbus plugin's QObject::destroyed is being connected 
to QDBusConnectionPrivate with Qt::BlockQueuedConnection. But at this point, 
QApplication is already gone and this call is never being handled. Disconnect 
everything in destructor can fix this issue.

Related upstream qt code:
https://github.com/qtproject/qtbase/blob/5.6.0/src/dbus/qdbusintegrator.cpp#L2135


Diffs
-

  src/backends/dbus/DBusHelperProxy.cpp 20dad0a 

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


Testing
---

Now kded5 can exits upon logout/exit. Tested with kquitapp5 kded5.


Thanks,

Xuetian Weng

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

Re: Review Request 127166: Fix xcb port of klauncher and clean up the code.

[Xuetian Weng]

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

(Updated March 4, 2016, 10:51 p.m.)


Review request for KDE Frameworks, David Faure and Martin Gräßlin.


Repository: kinit


Description
---

The ported klauncher at least has two bugs:
1. IMHO, it should compare the cached display string with == instead of != 
(though != has been there since the old kdelibs, but it just doesn't look 
correct to me, what's the point of assign dpy = mCached_dpy when dpy_str != 
XDisplayString(dpy)? And what's the point use == in one place and != in other 
two places? ).
2. it might free the cached connection without setting mCached.conn back to 
nullptr, which could lead to double free or freeze when system logout.

This patch refactor the code a little bit with a helper function to update the 
cached connection instead of duplicate the same logic everywhere. 
getXCBConnection() will make sure the returned connection is error-free, reuse 
the cached connection if possible, and update the cached connection if needed.


Diffs (updated)
-

  src/klauncher/klauncher.h 31bfaaa 
  src/klauncher/klauncher.cpp 7ea9da9 

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


Testing
---

Compiles, startup notify works.


Thanks,

Xuetian Weng

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

Re: Review Request 127261: Fix dead lock when program use kauth exits.

[Xuetian Weng]

> On March 3, 2016, 7:05 a.m., Martin Gräßlin wrote:
> > nice work! I had run into it in the past and didn't succeed investigating 
> > it.
> 
> Xuetian Weng wrote:
> https://bugreports.qt.io/browse/QTBUG-51648
> 
> As pointed out by Thiago on irc, this should be considered as a bug in 
> Qt. Not sure if we want to commit it. As disconnecting this signal indeed 
> breaks some invariant in qtdbus (not critical though, since we are going to 
> exit anyway).
> 
> Martin Gräßlin wrote:
> Let's wait then whether Qt 5.6 comes with it fixed or not.

Well, Thiago said there's no time to fix it before 5.6.0 though.


- Xuetian


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


On March 2, 2016, 8:58 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127261/
> ---
> 
> (Updated March 2, 2016, 8:58 p.m.)
> 
> 
> Review request for KDE Frameworks and David Edmundson.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> I'm on Qt 5.6 RC, and just notice a issue that kded5 never cleanly exits.
> 
> With gdb attached to kded5, it shows following backtrace:
> 
> #1  0x75c820eb in QWaitConditionPrivate::wait 
> (time=18446744073709551615, this=0x74ac70) at 
> thread/qwaitcondition_unix.cpp:136
> #2  QWaitCondition::wait (this=this@entry=0x71c308, 
> mutex=mutex@entry=0x71c300, time=time@entry=18446744073709551615) at 
> thread/qwaitcondition_unix.cpp:208
> #3  0x75c7b08b in QSemaphore::acquire 
> (this=this@entry=0x7fffd840, n=n@entry=1) at thread/qsemaphore.cpp:137
> #4  0x75e80fff in QMetaObject::activate 
> (sender=sender@entry=0x976280, signalOffset=, 
> local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x7fffd8d0)
> at kernel/qobject.cpp:3698
> #5  0x75e81687 in QMetaObject::activate 
> (sender=sender@entry=0x976280, m=m@entry=0x76092760 
> , local_signal_index=local_signal_index@entry=0, 
> argv=argv@entry=0x7fffd8d0) at kernel/qobject.cpp:3595
> #6  0x75e8172f in QObject::destroyed (this=this@entry=0x976280, 
> _t1=_t1@entry=0x976280) at .moc/moc_qobject.cpp:213
> #7  0x75e886e5 in QObject::~QObject (this=, 
> __in_chrg=) at kernel/qobject.cpp:913
> #8  0x7fffb39bbb09 in KAuth::DBusHelperProxy::~DBusHelperProxy 
> (this=0x976280, __in_chrg=) at 
> /chakra/core/kauth/src/kauth-5.19.0/src/backends/dbus/DBusHelperProxy.cpp:55
> #9  0x75e49ff9 in QLibraryPrivate::unload (this=0x71dba0, 
> flag=QLibraryPrivate::NoUnloadSys) at plugin/qlibrary.cpp:551
> #10 0x75e4e521 in QLibraryStore::cleanup () at 
> plugin/qlibrary.cpp:397
> #11 0x778592ef in __cxa_finalize () from /usr/lib/libc.so.6
> #12 0x75c52583 in __do_global_dtors_aux () from 
> /usr/lib/libQt5Core.so.5
> #13 0x7fffe230 in ?? ()
> #14 0x77dea867 in _dl_fini () from /lib64/ld-linux-x86-64.so.2
> 
> 
> Which indicates that kauth dbus plugin's QObject::destroyed is being 
> connected to QDBusConnectionPrivate with Qt::BlockQueuedConnection. But at 
> this point, QApplication is already gone and this call is never being 
> handled. Disconnect everything in destructor can fix this issue.
> 
> Related upstream qt code:
> https://github.com/qtproject/qtbase/blob/5.6.0/src/dbus/qdbusintegrator.cpp#L2135
> 
> 
> Diffs
> -
> 
>   src/backends/dbus/DBusHelperProxy.cpp 20dad0a 
> 
> Diff: https://git.reviewboard.kde.org/r/127261/diff/
> 
> 
> Testing
> ---
> 
> Now kded5 can exits upon logout/exit. Tested with kquitapp5 kded5.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 127261: Fix dead lock when program use kauth exits.

[Xuetian Weng]

> On March 3, 2016, 7:05 a.m., Martin Gräßlin wrote:
> > nice work! I had run into it in the past and didn't succeed investigating 
> > it.

https://bugreports.qt.io/browse/QTBUG-51648

As pointed out by Thiago on irc, this should be considered as a bug in Qt. Not 
sure if we want to commit it. As disconnecting this signal indeed breaks some 
invariant in qtdbus (not critical though, since we are going to exit anyway).


- Xuetian


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


On March 2, 2016, 8:58 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127261/
> ---
> 
> (Updated March 2, 2016, 8:58 p.m.)
> 
> 
> Review request for KDE Frameworks and David Edmundson.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> I'm on Qt 5.6 RC, and just notice a issue that kded5 never cleanly exits.
> 
> With gdb attached to kded5, it shows following backtrace:
> 
> #1  0x75c820eb in QWaitConditionPrivate::wait 
> (time=18446744073709551615, this=0x74ac70) at 
> thread/qwaitcondition_unix.cpp:136
> #2  QWaitCondition::wait (this=this@entry=0x71c308, 
> mutex=mutex@entry=0x71c300, time=time@entry=18446744073709551615) at 
> thread/qwaitcondition_unix.cpp:208
> #3  0x75c7b08b in QSemaphore::acquire 
> (this=this@entry=0x7fffd840, n=n@entry=1) at thread/qsemaphore.cpp:137
> #4  0x75e80fff in QMetaObject::activate 
> (sender=sender@entry=0x976280, signalOffset=, 
> local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x7fffd8d0)
> at kernel/qobject.cpp:3698
> #5  0x75e81687 in QMetaObject::activate 
> (sender=sender@entry=0x976280, m=m@entry=0x76092760 
> , local_signal_index=local_signal_index@entry=0, 
> argv=argv@entry=0x7fffd8d0) at kernel/qobject.cpp:3595
> #6  0x75e8172f in QObject::destroyed (this=this@entry=0x976280, 
> _t1=_t1@entry=0x976280) at .moc/moc_qobject.cpp:213
> #7  0x75e886e5 in QObject::~QObject (this=, 
> __in_chrg=) at kernel/qobject.cpp:913
> #8  0x7fffb39bbb09 in KAuth::DBusHelperProxy::~DBusHelperProxy 
> (this=0x976280, __in_chrg=) at 
> /chakra/core/kauth/src/kauth-5.19.0/src/backends/dbus/DBusHelperProxy.cpp:55
> #9  0x75e49ff9 in QLibraryPrivate::unload (this=0x71dba0, 
> flag=QLibraryPrivate::NoUnloadSys) at plugin/qlibrary.cpp:551
> #10 0x75e4e521 in QLibraryStore::cleanup () at 
> plugin/qlibrary.cpp:397
> #11 0x778592ef in __cxa_finalize () from /usr/lib/libc.so.6
> #12 0x75c52583 in __do_global_dtors_aux () from 
> /usr/lib/libQt5Core.so.5
> #13 0x7fffe230 in ?? ()
> #14 0x77dea867 in _dl_fini () from /lib64/ld-linux-x86-64.so.2
> 
> 
> Which indicates that kauth dbus plugin's QObject::destroyed is being 
> connected to QDBusConnectionPrivate with Qt::BlockQueuedConnection. But at 
> this point, QApplication is already gone and this call is never being 
> handled. Disconnect everything in destructor can fix this issue.
> 
> Related upstream qt code:
> https://github.com/qtproject/qtbase/blob/5.6.0/src/dbus/qdbusintegrator.cpp#L2135
> 
> 
> Diffs
> -
> 
>   src/backends/dbus/DBusHelperProxy.cpp 20dad0a 
> 
> Diff: https://git.reviewboard.kde.org/r/127261/diff/
> 
> 
> Testing
> ---
> 
> Now kded5 can exits upon logout/exit. Tested with kquitapp5 kded5.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Review Request 127271: Disable session restore for kwalletd5

[Xuetian Weng]

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

Review request for KDE Frameworks and Martin Klapetek.


Repository: kwallet


Description
---

I notice a kwalletd5 with "-session ." in its command line started on my 
desktop and kwallet-pam doesn't work.

Also kwalletd is dbus activated in other cases there's no point to let session 
manager to restore it.


Diffs
-

  src/runtime/kwalletd/main.cpp 740e670 

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


Testing
---

kwallet-pam back to work.


Thanks,

Xuetian Weng

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

Re: Review Request 127261: Fix dead lock when program use kauth exits.

[Xuetian Weng]

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

(Updated March 2, 2016, 8:58 p.m.)


Review request for KDE Frameworks and David Edmundson.


Repository: kauth


Description (updated)
---

I'm on Qt 5.6 RC, and just notice a issue that kded5 never cleanly exits.

With gdb attached to kded5, it shows following backtrace:

#1  0x75c820eb in QWaitConditionPrivate::wait 
(time=18446744073709551615, this=0x74ac70) at thread/qwaitcondition_unix.cpp:136
#2  QWaitCondition::wait (this=this@entry=0x71c308, 
mutex=mutex@entry=0x71c300, time=time@entry=18446744073709551615) at 
thread/qwaitcondition_unix.cpp:208
#3  0x75c7b08b in QSemaphore::acquire 
(this=this@entry=0x7fffd840, n=n@entry=1) at thread/qsemaphore.cpp:137
#4  0x75e80fff in QMetaObject::activate 
(sender=sender@entry=0x976280, signalOffset=, 
local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x7fffd8d0)
at kernel/qobject.cpp:3698
#5  0x75e81687 in QMetaObject::activate 
(sender=sender@entry=0x976280, m=m@entry=0x76092760 
, local_signal_index=local_signal_index@entry=0, 
argv=argv@entry=0x7fffd8d0) at kernel/qobject.cpp:3595
#6  0x75e8172f in QObject::destroyed (this=this@entry=0x976280, 
_t1=_t1@entry=0x976280) at .moc/moc_qobject.cpp:213
#7  0x75e886e5 in QObject::~QObject (this=, 
__in_chrg=) at kernel/qobject.cpp:913
#8  0x7fffb39bbb09 in KAuth::DBusHelperProxy::~DBusHelperProxy 
(this=0x976280, __in_chrg=) at 
/chakra/core/kauth/src/kauth-5.19.0/src/backends/dbus/DBusHelperProxy.cpp:55
#9  0x75e49ff9 in QLibraryPrivate::unload (this=0x71dba0, 
flag=QLibraryPrivate::NoUnloadSys) at plugin/qlibrary.cpp:551
#10 0x75e4e521 in QLibraryStore::cleanup () at 
plugin/qlibrary.cpp:397
#11 0x778592ef in __cxa_finalize () from /usr/lib/libc.so.6
#12 0x75c52583 in __do_global_dtors_aux () from 
/usr/lib/libQt5Core.so.5
#13 0x7fffe230 in ?? ()
#14 0x77dea867 in _dl_fini () from /lib64/ld-linux-x86-64.so.2


Which indicates that kauth dbus plugin's QObject::destroyed is being connected 
to QDBusConnectionPrivate with Qt::BlockQueuedConnection. But at this point, 
QApplication is already gone and this call is never being handled. Disconnect 
everything in destructor can fix this issue.

Related upstream qt code:
https://github.com/qtproject/qtbase/blob/5.6.0/src/dbus/qdbusintegrator.cpp#L2135


Diffs
-

  src/backends/dbus/DBusHelperProxy.cpp 20dad0a 

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


Testing
---

Now kded5 can exits upon logout/exit. Tested with kquitapp5 kded5.


Thanks,

Xuetian Weng

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

Review Request 127261: Fix dead lock when program use kauth exits.

[Xuetian Weng]

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

Review request for KDE Frameworks and David Edmundson.


Repository: kauth


Description
---

I'm on Qt 5.6 RC, and just notice a issue that kded5 never cleanly exits.

With gdb attached to kded5, it shows following backtrace:

#1  0x75c820eb in QWaitConditionPrivate::wait 
(time=18446744073709551615, this=0x74ac70) at thread/qwaitcondition_unix.cpp:136
#2  QWaitCondition::wait (this=this@entry=0x71c308, mutex=mutex@entry=0x71c300, 
time=time@entry=18446744073709551615) at thread/qwaitcondition_unix.cpp:208
#3  0x75c7b08b in QSemaphore::acquire (this=this@entry=0x7fffd840, 
n=n@entry=1) at thread/qsemaphore.cpp:137
#4  0x75e80fff in QMetaObject::activate (sender=sender@entry=0x976280, 
signalOffset=, local_signal_index=local_signal_index@entry=0, 
argv=argv@entry=0x7fffd8d0)
at kernel/qobject.cpp:3698
#5  0x75e81687 in QMetaObject::activate (sender=sender@entry=0x976280, 
m=m@entry=0x76092760 , 
local_signal_index=local_signal_index@entry=0, 
argv=argv@entry=0x7fffd8d0) at kernel/qobject.cpp:3595
#6  0x75e8172f in QObject::destroyed (this=this@entry=0x976280, 
_t1=_t1@entry=0x976280) at .moc/moc_qobject.cpp:213
#7  0x75e886e5 in QObject::~QObject (this=, 
__in_chrg=) at kernel/qobject.cpp:913
#8  0x7fffb39bbb09 in KAuth::DBusHelperProxy::~DBusHelperProxy 
(this=0x976280, __in_chrg=) at 
/chakra/core/kauth/src/kauth-5.19.0/src/backends/dbus/DBusHelperProxy.cpp:55
#9  0x75e49ff9 in QLibraryPrivate::unload (this=0x71dba0, 
flag=QLibraryPrivate::NoUnloadSys) at plugin/qlibrary.cpp:551
#10 0x75e4e521 in QLibraryStore::cleanup () at plugin/qlibrary.cpp:397
#11 0x778592ef in __cxa_finalize () from /usr/lib/libc.so.6
#12 0x75c52583 in __do_global_dtors_aux () from /usr/lib/libQt5Core.so.5
#13 0x7fffe230 in ?? ()
#14 0x77dea867 in _dl_fini () from /lib64/ld-linux-x86-64.so.2


Which indicates that kauth dbus plugin's QObject::destroyed is being connected 
to QDBusConnectionPrivate with Qt::BlockQueuedConnection. But at this point, 
QApplication is already gone and this call is never being handled. Disconnect 
everything in destructor can fix this issue.

Related upstream qt code:
https://github.com/qtproject/qtbase/blob/5.6.0/src/dbus/qdbusintegrator.cpp#L2135


Diffs
-

  src/backends/dbus/DBusHelperProxy.cpp 20dad0a 

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


Testing
---

Now kded5 can exits upon logout/exit. Tested with kquitapp5 kded5.


Thanks,

Xuetian Weng

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

Re: Review Request 127201: Fix svg icon path resolving in IconItem

[Xuetian Weng]

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

(Updated Feb. 29, 2016, 11:05 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
Marco Martin.


Changes
---

Submitted with commit 066cf0a0d7b46c60fccb945dbc0e9f858c4d6967 by Weng Xuetian 
to branch master.


Repository: plasma-framework


Description
---

Well.. the bug should be obvious:
1. QString iconPath overrides the iconPath in a higher scope. introduced in 
https://git.reviewboard.kde.org/r/126168/
2. iconPath not assigned, also introduced in 
https://git.reviewboard.kde.org/r/126168/
3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/


Diffs
-

  autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
  autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
  autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
PRE-CREATION 
  autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
PRE-CREATION 
  autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
  autotests/data/icons/test-theme/index.theme PRE-CREATION 
  autotests/iconitemtest.h 9fd3063 
  autotests/iconitemtest.cpp ea98c9d 
  src/declarativeimports/core/iconitem.cpp c65a9a7 

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


Testing
---

tested with qmlscene with Plasma.IconItem.


Thanks,

Xuetian Weng

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

Re: Review Request 127201: Fix svg icon path resolving in IconItem

[Xuetian Weng]

> On Feb. 28, 2016, 9:58 a.m., David Rosca wrote:
> > autotests/iconitemtest.cpp, line 234
> > <https://git.reviewboard.kde.org/r/127201/diff/2/?file=445755#file445755line234>
> >
> > Why?
> 
> Xuetian Weng wrote:
> Actually this check is not so reliable if the icon is not rendered with 
> the same svg backend but it will pass for now.
> 
> David Rosca wrote:
> But both items use exactly the same icon, so they should be rendered the 
> same. Do you have example when it fails?
> 
> Xuetian Weng wrote:
> You could try to move bug359388.svg to data/icons/test-theme/apps/22/ , 
> and comment out the QSKIP.
> 
> I guess the problem is that it might just render a svg scaled in some 
> cases when using QIcon, in some other cases, it will just render the svg 
> directly to the correct size.

Ah, ok , the problem here is my test environment has QT_DEVICE_PIXEL_RATIO set.


- Xuetian


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


On Feb. 28, 2016, 5:15 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127201/
> ---
> 
> (Updated Feb. 28, 2016, 5:15 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
> Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Well.. the bug should be obvious:
> 1. QString iconPath overrides the iconPath in a higher scope. introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 2. iconPath not assigned, also introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/
> 
> 
> Diffs
> -
> 
>   autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
>   autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
>   autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
>   autotests/data/icons/test-theme/index.theme PRE-CREATION 
>   autotests/iconitemtest.h 9fd3063 
>   autotests/iconitemtest.cpp ea98c9d 
>   src/declarativeimports/core/iconitem.cpp c65a9a7 
> 
> Diff: https://git.reviewboard.kde.org/r/127201/diff/
> 
> 
> Testing
> ---
> 
> tested with qmlscene with Plasma.IconItem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 127201: Fix svg icon path resolving in IconItem

[Xuetian Weng]

> On Feb. 28, 2016, 9:58 a.m., David Rosca wrote:
> > autotests/iconitemtest.cpp, line 234
> > <https://git.reviewboard.kde.org/r/127201/diff/2/?file=445755#file445755line234>
> >
> > Why?
> 
> Xuetian Weng wrote:
> Actually this check is not so reliable if the icon is not rendered with 
> the same svg backend but it will pass for now.
> 
> David Rosca wrote:
> But both items use exactly the same icon, so they should be rendered the 
> same. Do you have example when it fails?

You could try to move bug359388.svg to data/icons/test-theme/apps/22/ , and 
comment out the QSKIP.

I guess the problem is that it might just render a svg scaled in some cases 
when using QIcon, in some other cases, it will just render the svg directly to 
the correct size.


- Xuetian


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


On Feb. 28, 2016, 5:15 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127201/
> ---
> 
> (Updated Feb. 28, 2016, 5:15 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
> Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Well.. the bug should be obvious:
> 1. QString iconPath overrides the iconPath in a higher scope. introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 2. iconPath not assigned, also introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/
> 
> 
> Diffs
> -
> 
>   autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
>   autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
>   autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
>   autotests/data/icons/test-theme/index.theme PRE-CREATION 
>   autotests/iconitemtest.h 9fd3063 
>   autotests/iconitemtest.cpp ea98c9d 
>   src/declarativeimports/core/iconitem.cpp c65a9a7 
> 
> Diff: https://git.reviewboard.kde.org/r/127201/diff/
> 
> 
> Testing
> ---
> 
> tested with qmlscene with Plasma.IconItem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 127201: Fix svg icon path resolving in IconItem

[Xuetian Weng]

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

(Updated Feb. 28, 2016, 5:15 p.m.)


Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
Marco Martin.


Changes
---

Fixes for David Rosca's comment


Repository: plasma-framework


Description
---

Well.. the bug should be obvious:
1. QString iconPath overrides the iconPath in a higher scope. introduced in 
https://git.reviewboard.kde.org/r/126168/
2. iconPath not assigned, also introduced in 
https://git.reviewboard.kde.org/r/126168/
3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/


Diffs (updated)
-

  autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
  autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
  autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
PRE-CREATION 
  autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
PRE-CREATION 
  autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
  autotests/data/icons/test-theme/index.theme PRE-CREATION 
  autotests/iconitemtest.h 9fd3063 
  autotests/iconitemtest.cpp ea98c9d 
  src/declarativeimports/core/iconitem.cpp c65a9a7 

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


Testing
---

tested with qmlscene with Plasma.IconItem.


Thanks,

Xuetian Weng

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

Re: Review Request 127201: Fix svg icon path resolving in IconItem

[Xuetian Weng]

> On Feb. 28, 2016, 9:58 a.m., David Rosca wrote:
> > autotests/iconitemtest.cpp, line 253
> > <https://git.reviewboard.kde.org/r/127201/diff/2/?file=445755#file445755line253>
> >
> > This should be `QSize(32, 32)` as the comment says?

I indented to use a size without exact match.


> On Feb. 28, 2016, 9:58 a.m., David Rosca wrote:
> > autotests/iconitemtest.cpp, line 234
> > <https://git.reviewboard.kde.org/r/127201/diff/2/?file=445755#file445755line234>
> >
> > Why?

Actually this check is not so reliable if the icon is not rendered with the 
same svg backend but it will pass for now.


- Xuetian


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


On Feb. 28, 2016, 1:18 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127201/
> ---
> 
> (Updated Feb. 28, 2016, 1:18 a.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
> Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Well.. the bug should be obvious:
> 1. QString iconPath overrides the iconPath in a higher scope. introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 2. iconPath not assigned, also introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/
> 
> 
> Diffs
> -
> 
>   autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
>   autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
>   autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
>   autotests/data/icons/test-theme/index.theme PRE-CREATION 
>   autotests/iconitemtest.h 9fd3063 
>   autotests/iconitemtest.cpp f675465 
>   src/declarativeimports/core/iconitem.cpp c65a9a7 
> 
> Diff: https://git.reviewboard.kde.org/r/127201/diff/
> 
> 
> Testing
> ---
> 
> tested with qmlscene with Plasma.IconItem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 127201: Fix svg icon path resolving in IconItem

[Xuetian Weng]

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

(Updated Feb. 28, 2016, 1:18 a.m.)


Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
Marco Martin.


Changes
---

Ah, it's really hard to make the test adapts custom path for icon, because 
KIconTheme::iconPath is used directly in icon item.

Also make the test not depending on breeze icon theme  by adding a konversation 
icon to the test-theme used here.
BTW, breeze plasma theme name is "default" not "breeze".


Repository: plasma-framework


Description
---

Well.. the bug should be obvious:
1. QString iconPath overrides the iconPath in a higher scope. introduced in 
https://git.reviewboard.kde.org/r/126168/
2. iconPath not assigned, also introduced in 
https://git.reviewboard.kde.org/r/126168/
3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/


Diffs (updated)
-

  autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
  autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
  autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
PRE-CREATION 
  autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
PRE-CREATION 
  autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
  autotests/data/icons/test-theme/index.theme PRE-CREATION 
  autotests/iconitemtest.h 9fd3063 
  autotests/iconitemtest.cpp f675465 
  src/declarativeimports/core/iconitem.cpp c65a9a7 

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


Testing
---

tested with qmlscene with Plasma.IconItem.


Thanks,

Xuetian Weng

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

Review Request 127201: Fix svg icon path resolving in IconItem

[Xuetian Weng]

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

Review request for KDE Frameworks, Plasma and Marco Martin.


Repository: plasma-framework


Description
---

Well.. the bug should be obvious:
1. QString iconPath overrides the iconPath in a higher scope. introduced in 
https://git.reviewboard.kde.org/r/126168/
2. iconPath not assigned, also introduced in 
https://git.reviewboard.kde.org/r/126168/
3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/


Diffs
-

  src/declarativeimports/core/iconitem.cpp 085f284 

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


Testing
---

tested with qmlscene with Plasma.IconItem.


Thanks,

Xuetian Weng

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

Re: Review Request 127166: Fix xcb port of klauncher and clean up the code.

[Xuetian Weng]

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



ping

- Xuetian Weng


On Feb. 24, 2016, 5:19 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127166/
> ---
> 
> (Updated Feb. 24, 2016, 5:19 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Martin Gräßlin.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> The ported klauncher at least has two bugs:
> 1. IMHO, it should compare the cached display string with == instead of != 
> (though != has been there since the old kdelibs, but it just doesn't look 
> correct to me, what's the point of assign dpy = mCached_dpy when dpy_str != 
> XDisplayString(dpy)? And what's the point use == in one place and != in other 
> two places? ).
> 2. it might free the cached connection without setting mCached.conn back to 
> nullptr, which could lead to double free or freeze when system logout.
> 
> This patch refactor the code a little bit with a helper function to update 
> the cached connection instead of duplicate the same logic everywhere. 
> getXCBConnection() will make sure the returned connection is error-free, 
> reuse the cached connection if possible, and update the cached connection if 
> needed.
> 
> 
> Diffs
> -
> 
>   src/klauncher/klauncher.h 31bfaaa 
>   src/klauncher/klauncher.cpp 7ea9da9 
> 
> Diff: https://git.reviewboard.kde.org/r/127166/diff/
> 
> 
> Testing
> ---
> 
> Compiles, startup notify works.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 127166: Fix xcb port of klauncher and clean up the code.

[Xuetian Weng]

> On Feb. 24, 2016, 7:19 a.m., Martin Gräßlin wrote:
> > src/klauncher/klauncher.h, line 274
> > <https://git.reviewboard.kde.org/r/127166/diff/1/?file=445338#file445338line274>
> >
> > const QByteArray &

Well, I assign to it in the function.


- Xuetian


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


On Feb. 24, 2016, 5:19 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127166/
> ---
> 
> (Updated Feb. 24, 2016, 5:19 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Martin Gräßlin.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> The ported klauncher at least has two bugs:
> 1. IMHO, it should compare the cached display string with == instead of != 
> (though != has been there since the old kdelibs, but it just doesn't look 
> correct to me, what's the point of assign dpy = mCached_dpy when dpy_str != 
> XDisplayString(dpy)? And what's the point use == in one place and != in other 
> two places? ).
> 2. it might free the cached connection without setting mCached.conn back to 
> nullptr, which could lead to double free or freeze when system logout.
> 
> This patch refactor the code a little bit with a helper function to update 
> the cached connection instead of duplicate the same logic everywhere. 
> getXCBConnection() will make sure the returned connection is error-free, 
> reuse the cached connection if possible, and update the cached connection if 
> needed.
> 
> 
> Diffs
> -
> 
>   src/klauncher/klauncher.h 31bfaaa 
>   src/klauncher/klauncher.cpp 7ea9da9 
> 
> Diff: https://git.reviewboard.kde.org/r/127166/diff/
> 
> 
> Testing
> ---
> 
> Compiles, startup notify works.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Review Request 127166: Fix xcb port of klauncher and clean up the code.

[Xuetian Weng]

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

Review request for KDE Frameworks, David Faure and Martin Gräßlin.


Repository: kinit


Description
---

The ported klauncher at least has two bugs:
1. IMHO, it should compare the cached display string with == instead of != 
(though != has been there since the old kdelibs, but it just doesn't look 
correct to me, what's the point of assign dpy = mCached_dpy when dpy_str != 
XDisplayString(dpy)? And what's the point use == in one place and != in other 
two places? ).
2. it might free the cached connection without setting mCached.conn back to 
nullptr, which could lead to double free or freeze when system logout.

This patch refactor the code a little bit with a helper function to update the 
cached connection instead of duplicate the same logic everywhere. 
getXCBConnection() will make sure the returned connection is error-free, reuse 
the cached connection if possible, and update the cached connection if needed.


Diffs
-

  src/klauncher/klauncher.h 31bfaaa 
  src/klauncher/klauncher.cpp 7ea9da9 

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


Testing
---

Compiles, startup notify works.


Thanks,

Xuetian Weng

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

Re: Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[Xuetian Weng]

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

(Updated Dec. 14, 2015, 11:31 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.


Changes
---

Submitted with commit d34b37b6831429f7a48edec7de77cb88524b3784 by Weng Xuetian 
to branch master.


Repository: kio


Description
---

9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on startIndex. 
The argument index passed to buttonUrl is based number of '/'in full url. 
Because of that, url like "sftp://a...@b.com/a/b"; is now shown as 
"sftp:a...@b.com" / "b" / "b" in dolphin url bar.

This patch changes all relevant code that calls buttonUrl() to use url.path() 
instead of url.toDisplayString() to count the number of "/".


Diffs
-

  src/filewidgets/kurlnavigator.cpp 848e98b 

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


Testing
---

Remote url -> ok
Local url -> ok
Remote url in places and try browse some sub folder -> ok
Local url in places and try browse some sub folder -> ok


Thanks,

Xuetian Weng

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

Re: Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[Xuetian Weng]

> On Dec. 12, 2015, 10:54 a.m., Emmanuel Pescosta wrote:
> > Sorry for breaking this code :/
> > 
> > Would it be possible to add some test cases to prevent such problems in 
> > future?

I don't know. It's not a good practice to test with private interface.


- Xuetian


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


On Dec. 12, 2015, 11:11 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126295/
> ---
> 
> (Updated Dec. 12, 2015, 11:11 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> 9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on 
> startIndex. The argument index passed to buttonUrl is based number of '/'in 
> full url. Because of that, url like "sftp://a...@b.com/a/b"; is now shown as 
> "sftp:a...@b.com" / "b" / "b" in dolphin url bar.
> 
> This patch changes all relevant code that calls buttonUrl() to use url.path() 
> instead of url.toDisplayString() to count the number of "/".
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kurlnavigator.cpp 848e98b 
> 
> Diff: https://git.reviewboard.kde.org/r/126295/diff/
> 
> 
> Testing
> ---
> 
> Remote url -> ok
> Local url -> ok
> Remote url in places and try browse some sub folder -> ok
> Local url in places and try browse some sub folder -> ok
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[Xuetian Weng]

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

(Updated Dec. 12, 2015, 11:11 a.m.)


Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.


Repository: kio


Description
---

9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on startIndex. 
The argument index passed to buttonUrl is based number of '/'in full url. 
Because of that, url like "sftp://a...@b.com/a/b"; is now shown as 
"sftp:a...@b.com" / "b" / "b" in dolphin url bar.

This patch changes all relevant code that calls buttonUrl() to use url.path() 
instead of url.toDisplayString() to count the number of "/".


Diffs (updated)
-

  src/filewidgets/kurlnavigator.cpp 848e98b 

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


Testing
---

Remote url -> ok
Local url -> ok
Remote url in places and try browse some sub folder -> ok
Local url in places and try browse some sub folder -> ok


Thanks,

Xuetian Weng

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

Re: Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[Xuetian Weng]

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

(Updated Dec. 12, 2015, 9:35 a.m.)


Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.


Repository: kio


Description
---

9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on startIndex. 
The argument index passed to buttonUrl is based number of '/'in full url. 
Because of that, url like "sftp://a...@b.com/a/b"; is now shown as 
"sftp:a...@b.com" / "b" / "b" in dolphin url bar.

This patch changes all relevant code that calls buttonUrl() to use url.path() 
instead of url.toDisplayString() to count the number of "/".


Diffs (updated)
-

  src/filewidgets/kurlnavigator.cpp 848e98b 

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


Testing
---

Remote url -> ok
Local url -> ok
Remote url in places and try browse some sub folder -> ok
Local url in places and try browse some sub folder -> ok


Thanks,

Xuetian Weng

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

Review Request 126295: Fix wrong button in KUrlNavigator issue caused by 9dbe36f734b5b839b2a6a934fad29d639e954498

[Xuetian Weng]

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

Review request for KDE Frameworks, David Faure and Emmanuel Pescosta.


Repository: kio


Description
---

9dbe36f734b5b839b2a6a934fad29d639e954498 breaks some assumption on startIndex. 
The argument index passed to buttonUrl is based number of '/'in full url. 
Because of that, url like "sftp://a...@b.com/a/b"; is now shown as 
"sftp:a...@b.com" / "b" / "b" in dolphin url bar.

This patch changes all relevant code that calls buttonUrl() to use url.path() 
instead of url.toDisplayString() to count the number of "/".


Diffs
-

  src/filewidgets/fix-url-navigator.patch PRE-CREATION 
  src/filewidgets/kurlnavigator.cpp 848e98b 

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


Testing
---

Remote url -> ok
Local url -> ok
Remote url in places and try browse some sub folder -> ok
Local url in places and try browse some sub folder -> ok


Thanks,

Xuetian Weng

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

Re: Review Request 126231: Fix a small artifact of kratingwidget on hidpi

[Xuetian Weng]

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

(Updated Dec. 6, 2015, 2:33 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Christoph Feck and David Edmundson.


Changes
---

Submitted with commit 100f8a3a63d5326176ff7355e1adb2133a748ed2 by Weng Xuetian 
to branch master.


Repository: kwidgetsaddons


Description
---

The other half of star size is wrong when calling drawPixmap. This change makes 
the size arguments same as other drawPixmap calls.


Diffs
-

  src/kratingpainter.cpp 1a4378a 

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


Testing
---

To reproduce the problem, try to set rating to 5 / 10 and make hover rating to 
be 6.
Now it draws correctly.


File Attachments


Demonstrate the problem
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/12/03/5fd4f10d-cc2a-49f4-b6b1-ac5ac1d1e9e5__screenshot24.png


Thanks,

Xuetian Weng

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

Re: Review Request 126222: Fix rating widget flickering when selecting rating with mouse

[Xuetian Weng]

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

(Updated Dec. 4, 2015, 4:26 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, David Faure and Lamarque Souza.


Changes
---

Submitted with commit efb14cb69ad53bcdb4f3f7b4f2439183d27ebb05 by Weng Xuetian 
to branch master.


Repository: kwidgetsaddons


Description
---

Refactor the rating calculation:
1. Disable feature introduced in bug 171343 when halfSteps is enabled.
2. Merge the two pieces of code, and try to make it clearer and simpler.
3. make hover 0 star also show visual effect.


Diffs
-

  src/kratingpainter.cpp 1a4378a 
  src/kratingwidget.cpp d0d2903 

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


Testing
---

Current rating is 0, hover on the 1st star, 1 star lights up, click on 1st 
star, rating becomes 2.

Current rating is 2. hover on the 1st star, half star lights up, click on 1st 
star, rating becomes 1.

Current rating is 1/2. move mouse to the left most edge, star's color becomes 
lighter. Click and rating becomes 0.


Thanks,

Xuetian Weng

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

Re: Review Request 126222: Fix rating widget flickering when selecting rating with mouse

[Xuetian Weng]

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

(Updated Dec. 3, 2015, 10:19 a.m.)


Review request for KDE Frameworks, David Faure and Lamarque Souza.


Changes
---

add an extra check to avoid rating to be set to -1.


Repository: kwidgetsaddons


Description
---

Refactor the rating calculation:
1. Disable feature introduced in bug 171343 when halfSteps is enabled.
2. Merge the two pieces of code, and try to make it clearer and simpler.
3. make hover 0 star also show visual effect.


Diffs (updated)
-

  src/kratingpainter.cpp 1a4378a 
  src/kratingwidget.cpp d0d2903 

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


Testing
---

Current rating is 0, hover on the 1st star, 1 star lights up, click on 1st 
star, rating becomes 2.

Current rating is 2. hover on the 1st star, half star lights up, click on 1st 
star, rating becomes 1.

Current rating is 1/2. move mouse to the left most edge, star's color becomes 
lighter. Click and rating becomes 0.


Thanks,

Xuetian Weng

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

Re: Review Request 126222: Fix rating widget flickering when selecting rating with mouse

[Xuetian Weng]

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

(Updated Dec. 3, 2015, 9:56 a.m.)


Review request for KDE Frameworks, David Faure and Lamarque Souza.


Changes
---

upload the actual patch.


Repository: kwidgetsaddons


Description
---

Refactor the rating calculation:
1. Disable feature introduced in bug 171343 when halfSteps is enabled.
2. Merge the two pieces of code, and try to make it clearer and simpler.
3. make hover 0 star also show visual effect.


Diffs (updated)
-

  src/kratingpainter.cpp 1a4378a 
  src/kratingwidget.cpp d0d2903 

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


Testing
---

Current rating is 0, hover on the 1st star, 1 star lights up, click on 1st 
star, rating becomes 2.

Current rating is 2. hover on the 1st star, half star lights up, click on 1st 
star, rating becomes 1.

Current rating is 1/2. move mouse to the left most edge, star's color becomes 
lighter. Click and rating becomes 0.


Thanks,

Xuetian Weng

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

Review Request 126231: Fix a small artifact of kratingwidget on hidpi

[Xuetian Weng]

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

Review request for KDE Frameworks, Christoph Feck and David Edmundson.


Repository: kwidgetsaddons


Description
---

The other half of star size is wrong when calling drawPixmap. This change makes 
the size arguments same as other drawPixmap calls.


Diffs
-

  src/kratingpainter.cpp 1a4378a 

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


Testing
---

To reproduce the problem, try to set rating to 5 / 10 and make hover rating to 
be 6.
Now it draws correctly.


File Attachments


Demonstrate the problem
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/12/03/5fd4f10d-cc2a-49f4-b6b1-ac5ac1d1e9e5__screenshot24.png


Thanks,

Xuetian Weng

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

Re: Review Request 126222: Fix rating widget flickering when selecting rating with mouse

[Xuetian Weng]

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

(Updated Dec. 2, 2015, 11:37 p.m.)


Review request for KDE Frameworks and David Faure.


Repository: kwidgetsaddons


Description (updated)
---

Refactor the rating calculation:
1. Disable feature introduced in bug 171343 when halfSteps is enabled.
2. Merge the two pieces of code, and try to make it clearer and simpler.
3. make hover 0 star also show visual effect.


Diffs (updated)
-

  src/kratingwidget.cpp d0d2903 

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


Testing (updated)
---

Current rating is 0, hover on the 1st star, 1 star lights up, click on 1st 
star, rating becomes 2.

Current rating is 2. hover on the 1st star, half star lights up, click on 1st 
star, rating becomes 1.

Current rating is 1/2. move mouse to the left most edge, star's color becomes 
lighter. Click and rating becomes 0.


Thanks,

Xuetian Weng

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

Re: Review Request 126222: Fix rating widget flickering when selecting rating with mouse

[Xuetian Weng]

> On Dec. 2, 2015, 4:45 p.m., Lamarque Souza wrote:
> > That code was added to fix this bug 
> > https://bugs.kde.org/show_bug.cgi?id=171343#c6

Emm.. interesting feature. But code inside mouseMoveEvent looks fishy, and this 
feature shouldn't be used when halfStepsEnabled is true. I'll try to keep this 
feature then.


- Xuetian


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


On Dec. 2, 2015, 4:34 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126222/
> ---
> 
> (Updated Dec. 2, 2015, 4:34 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> mousePressed and mouseMoved seems to contains some hack code. I don't know 
> what these hacks are originally for, but it seems that they are not valid 
> anymore and causes problem.
> 
> Currently, whenever a mouse move happens, the rating value cycles between 0, 
> 1, 2. This change drops all hack code.
> 
> 
> Diffs
> -
> 
>   src/kratingwidget.cpp d0d2903 
> 
> Diff: https://git.reviewboard.kde.org/r/126222/diff/
> 
> 
> Testing
> ---
> 
> No flickering anymore when selecting rating.
> Tested with following case, all cases are able to select all possible rating 
> value.
> setMaxRating(2) setHalfStepsEnabled(false)
> setMaxRating(1) setHalfStepsEnabled(false)
> setMaxRating(1) setHalfStepsEnabled(true)
> setMaxRating(5) setHalfStepsEnabled(true)
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Review Request 126222: Fix rating widget flickering when selecting rating with mouse

[Xuetian Weng]

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

Review request for KDE Frameworks and David Faure.


Repository: kwidgetsaddons


Description
---

mousePressed and mouseMoved seems to contains some hack code. I don't know what 
these hacks are originally for, but it seems that they are not valid anymore 
and causes problem.

Currently, whenever a mouse move happens, the rating value cycles between 0, 1, 
2. This change drops all hack code.


Diffs
-

  src/kratingwidget.cpp d0d2903 

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


Testing
---

No flickering anymore when selecting rating.
Tested with following case, all cases are able to select all possible rating 
value.
setMaxRating(2) setHalfStepsEnabled(false)
setMaxRating(1) setHalfStepsEnabled(false)
setMaxRating(1) setHalfStepsEnabled(true)
setMaxRating(5) setHalfStepsEnabled(true)


Thanks,

Xuetian Weng

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

Re: Review Request 126164: Request proper dbus name for kioexec

[Xuetian Weng]

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

(Updated Nov. 25, 2015, 8:35 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit 1b4289ca0e00bf7ba59b19837ea2b2519f42a984 by Weng Xuetian 
to branch master.


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


Repository: kio


Description
---

Dolphin make a synchronous calls to klauncher and klauncher asks kdeinit to 
start kioexec. The call is only replied when kioexec's dbus name appear on the 
dbus.

Possibly in kde it uses kapplication and kdbusservice name is not added when 
porting to qt5/kf5.


Diffs
-

  src/kioexec/CMakeLists.txt 91284a3 
  src/kioexec/main.cpp 68ed374 

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


Testing
---

Dolphin now does not freeze in this case.


Thanks,

Xuetian Weng

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

Review Request 126164: Request proper dbus name for kioexec

[Xuetian Weng]

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

Review request for KDE Frameworks and David Faure.


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


Repository: kio


Description
---

Dolphin make a synchronous calls to klauncher and klauncher asks kdeinit to 
start kioexec. The call is only replied when kioexec's dbus name appear on the 
dbus.

Possibly in kde it uses kapplication and kdbusservice name is not added when 
porting to qt5/kf5.


Diffs
-

  src/kioexec/CMakeLists.txt 91284a3 
  src/kioexec/main.cpp 68ed374 

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


Testing
---

Dolphin now does not freeze in this case.


Thanks,

Xuetian Weng

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

Re: Review Request 125885: Support socks5 proxy in KTcpSocket

[Xuetian Weng]

> On Nov. 2, 2015, 8:08 a.m., David Faure wrote:
> > Ah I see, forget what I said about HTTP proxy.
> > 
> > About the missing argument, it would indeed be useful for this code:
> > http://lxr.kde.org/source/extragear/network/konversation/src/irc/server.cpp
> > Add a separate method, for BC reasons.

After you mentioned it, I actually test the http proxy (proxy server I used is 
polipo) and pure tcp, and it does support tcp connection over http proxy.

I'd like to be more careful for other users of ktcpsocket in ioslaves.

KTcpSocket does not know the protocol that this TcpSocket will use. But http 
ioslave uses setApplicationProxy to set the proxy. Use connectionHost with 
policy = AutoProxy may make http uses socks instead http proxy by default. Same 
applies to ftp ioslave and maybe others ioslave use TCPSlaveBase.

I wonder if there should also be some protocol hint argument of connectHost, or 
http and ftp ioslave should use policy = ManualProxy.


- Xuetian


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


On Oct. 30, 2015, 11:26 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125885/
> ---
> 
> (Updated Oct. 30, 2015, 11:26 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 342402
> https://bugs.kde.org/show_bug.cgi?id=342402
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Automatically set socks5 proxy in KTcpSocket.
> 
> 
> Diffs
> -
> 
>   src/core/ktcpsocket.h ffa3f0b 
>   src/core/ktcpsocket.cpp fde35a7 
> 
> Diff: https://git.reviewboard.kde.org/r/125885/diff/
> 
> 
> Testing
> ---
> 
> Test with akonadi imap agent, connect through socks5 proxy.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 125904: Save proxy url with correct scheme.

[Xuetian Weng]

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

(Updated Nov. 8, 2015, 3:26 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Eike Hein.


Changes
---

Submitted with commit 4e2a52c67167507b62553ca595bb561b925dc440 by Weng Xuetian 
to branch master.


Repository: kio


Description
---

Currently, if user type a ip address/domain name in socks 5 proxy field, the 
saved proxy url in configuration file will start with http:// instead of 
socks:// . Same applies to ftp proxy.

The reason is that KUrlFilter append http scheme to url without scheme. This 
patch uses setDefaultUrlScheme to make sure the correct scheme is used.

Also make necessary change to make sure that loading url configuration will 
hide url scheme correctly.


Diffs
-

  src/kcms/kio/kproxydlg.cpp c8fd0cd 

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


Testing
---

Save socks proxy and ftp proxy will result in "ftp://"; and "socks://".


Thanks,

Xuetian Weng

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

Re: Review Request 125904: Save proxy url with correct scheme.

[Xuetian Weng]

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

(Updated Nov. 3, 2015, 3:48 p.m.)


Review request for KDE Frameworks and Eike Hein.


Repository: kio


Description
---

Currently, if user type a ip address/domain name in socks 5 proxy field, the 
saved proxy url in configuration file will start with http:// instead of 
socks:// . Same applies to ftp proxy.

The reason is that KUrlFilter append http scheme to url without scheme. This 
patch uses setDefaultUrlScheme to make sure the correct scheme is used.

Also make necessary change to make sure that loading url configuration will 
hide url scheme correctly.


Diffs (updated)
-

  src/kcms/kio/kproxydlg.cpp c8fd0cd 

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


Testing
---

Save socks proxy and ftp proxy will result in "ftp://"; and "socks://".


Thanks,

Xuetian Weng

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

Review Request 125904: Save proxy url with correct scheme.

[Xuetian Weng]

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

Review request for KDE Frameworks and Eike Hein.


Repository: kio


Description
---

Currently, if user type a ip address/domain name in socks 5 proxy field, the 
saved proxy url in configuration file will start with http:// instead of 
socks:// . Same applies to ftp proxy.

The reason is that KUrlFilter append http scheme to url without scheme. This 
patch uses setDefaultUrlScheme to make sure the correct scheme is used.

Also make necessary change to make sure that loading url configuration will 
hide url scheme correctly.


Diffs
-

  src/kcms/kio/kproxydlg.cpp c8fd0cd 

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


Testing
---

Save socks proxy and ftp proxy will result in "ftp://"; and "socks://".


Thanks,

Xuetian Weng

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

Re: Review Request 125885: Support socks5 proxy in KTcpSocket

[Xuetian Weng]

> On Oct. 31, 2015, 8:39 a.m., David Faure wrote:
> > This is clearly missing an "else" for other cases like an http proxy, can 
> > you add it, even without being able to test it?
> > 
> > In any case this is a clear improvement.
> 
> Xuetian Weng wrote:
> I'm not aware that http proxy can handle arbitrary tcp connection :| .. 
> is it supported?

Another thing is that connectToHostEncrypted doesn't have ProxyPolicy argument 
in current API comparing with connectToHost. Does a new function need to be 
added?


- Xuetian


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


On Oct. 30, 2015, 11:26 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125885/
> ---
> 
> (Updated Oct. 30, 2015, 11:26 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 342402
> https://bugs.kde.org/show_bug.cgi?id=342402
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Automatically set socks5 proxy in KTcpSocket.
> 
> 
> Diffs
> -
> 
>   src/core/ktcpsocket.h ffa3f0b 
>   src/core/ktcpsocket.cpp fde35a7 
> 
> Diff: https://git.reviewboard.kde.org/r/125885/diff/
> 
> 
> Testing
> ---
> 
> Test with akonadi imap agent, connect through socks5 proxy.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request 125885: Support socks5 proxy in KTcpSocket

[Xuetian Weng]

> On Oct. 31, 2015, 8:39 a.m., David Faure wrote:
> > This is clearly missing an "else" for other cases like an http proxy, can 
> > you add it, even without being able to test it?
> > 
> > In any case this is a clear improvement.

I'm not aware that http proxy can handle arbitrary tcp connection :| .. is it 
supported?


- Xuetian


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


On Oct. 30, 2015, 11:26 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125885/
> ---
> 
> (Updated Oct. 30, 2015, 11:26 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 342402
> https://bugs.kde.org/show_bug.cgi?id=342402
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Automatically set socks5 proxy in KTcpSocket.
> 
> 
> Diffs
> -
> 
>   src/core/ktcpsocket.h ffa3f0b 
>   src/core/ktcpsocket.cpp fde35a7 
> 
> Diff: https://git.reviewboard.kde.org/r/125885/diff/
> 
> 
> Testing
> ---
> 
> Test with akonadi imap agent, connect through socks5 proxy.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Review Request 125885: Support socks5 proxy in KTcpSocket

[Xuetian Weng]

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

Review request for KDE Frameworks and David Faure.


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


Repository: kio


Description
---

Automatically set socks5 proxy in KTcpSocket.


Diffs
-

  src/core/ktcpsocket.h ffa3f0b 
  src/core/ktcpsocket.cpp fde35a7 

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


Testing
---

Test with akonadi imap agent, connect through socks5 proxy.


Thanks,

Xuetian Weng

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

Re: Review Request 125705: Use LANG for month names in calendar applet

[Xuetian Weng]

> On Oct. 20, 2015, 9:30 p.m., Albert Astals Cid wrote:
> > src/declarativeimports/calendar/calendar.cpp, line 197
> > 
> >
> > uilanguages is not good for direct inputing into QLocale (yes i know, 
> > it's stupid).
> > 
> > See how uilanguages returns en-US and QLocale wants en_US

Actually QLocale has no problem parsing things like en-US. See implementation 
https://github.com/qtproject/qtbase/blob/dev/src/corelib/tools/qlocale.cpp#L401 
, the separator contains both _ and -. 

uiLanguages() tend to return standard bcp 47 format, thus I don't think the 
implementation will become stricter.


- Xuetian


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


On Oct. 20, 2015, 7:04 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125705/
> ---
> 
> (Updated Oct. 20, 2015, 7:04 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Bugs: 353715
> http://bugs.kde.org/show_bug.cgi?id=353715
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Simple QDate::longMonthName won't do the job as it
> will return the month name using LC_DATE locale which is used
> for date formatting etc. So for example, in en_US locale
> and cs_CZ LC_DATE, it would return Czech month names while
> it should return English ones. So here we force the LANG
> locale and take the month name from that.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/calendar/calendar.cpp 67a08e5 
> 
> Diff: https://git.reviewboard.kde.org/r/125705/diff/
> 
> 
> Testing
> ---
> 
> LC_DATE=cs_CZ
> LANG=en_US
> 
> Month names in calendar applet are now english.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Re: Review Request 125705: Use LANG for month names in calendar applet

[Xuetian Weng]

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



src/declarativeimports/calendar/calendar.cpp (line 195)
<https://git.reviewboard.kde.org/r/125705/#comment59864>

This only checks LANG, meanwhile, LC_ALL and LC_MESSAGES, and maybe even 
LANGUAGE should also be considered.

But I guess using the first item from QLocale().uiLanguages() would just 
work.


- Xuetian Weng


On Oct. 19, 2015, 4:13 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125705/
> ---
> 
> (Updated Oct. 19, 2015, 4:13 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Bugs: 353715
> http://bugs.kde.org/show_bug.cgi?id=353715
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Simple QDate::longMonthName won't do the job as it
> will return the month name using LC_DATE locale which is used
> for date formatting etc. So for example, in en_US locale
> and cs_CZ LC_DATE, it would return Czech month names while
> it should return English ones. So here we force the LANG
> locale and take the month name from that.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/calendar/calendar.cpp 67a08e5 
> 
> Diff: https://git.reviewboard.kde.org/r/125705/diff/
> 
> 
> Testing
> ---
> 
> LC_DATE=cs_CZ
> LANG=en_US
> 
> Month names in calendar applet are now english.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Re: Review Request 125537: Use largest timestamp in subdirectory as resource directory timestamp.

[Xuetian Weng]

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

(Updated Oct. 6, 2015, 9:42 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Albert Astals Cid and David Faure.


Changes
---

Submitted with commit 0a39ba6ec01600efeb9e334fa457dd725e2e3749 by David Faure 
on behalf of Xuetian Weng to branch master.


Repository: kservice


Description
---

In 0e4d7247e27f82a9b79b19dbceb35b603baceb76, a new feature saving directory 
mtime in ksycoca db is introduced.

In the related code, TimestampChecker() will check the mtime recursively, but 
the building process will only read the top level directories' mtime, which 
causes ksycoca db being recreated again and again.

I hit this because my ~/.config/menus/applications-merged mtime newer than 
~/.config/menus. And plasmashell is running with 100% since it's creating 
ksycoca db repeatedly.

This patch change the building db procedure to use the same logic as in 
TimestampChecker(), so building and checking are consistent now.


Diffs
-

  autotests/ksycocatest.cpp 67f0700 
  src/sycoca/kbuildsycoca.cpp 00aaac3 
  src/sycoca/ksycoca.cpp 5209389 
  src/sycoca/ksycocautils_p.h c14057b 

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


Testing
---

Now a sub directory with larger timestamp than parent will not cause ksycoca 
data being recreated again and again.


Thanks,

Xuetian Weng

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

Re: Review Request 125537: Use largest timestamp in subdirectory as resource directory timestamp.

[Xuetian Weng]

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

(Updated Oct. 6, 2015, 11:06 a.m.)


Review request for KDE Frameworks, Albert Astals Cid and David Faure.


Repository: kservice


Description
---

In 0e4d7247e27f82a9b79b19dbceb35b603baceb76, a new feature saving directory 
mtime in ksycoca db is introduced.

In the related code, TimestampChecker() will check the mtime recursively, but 
the building process will only read the top level directories' mtime, which 
causes ksycoca db being recreated again and again.

I hit this because my ~/.config/menus/applications-merged mtime newer than 
~/.config/menus. And plasmashell is running with 100% since it's creating 
ksycoca db repeatedly.

This patch change the building db procedure to use the same logic as in 
TimestampChecker(), so building and checking are consistent now.


Diffs (updated)
-

  autotests/ksycocatest.cpp 67f0700 
  src/sycoca/kbuildsycoca.cpp 00aaac3 
  src/sycoca/ksycoca.cpp 5209389 
  src/sycoca/ksycocautils_p.h c14057b 

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


Testing
---

Now a sub directory with larger timestamp than parent will not cause ksycoca 
data being recreated again and again.


Thanks,

Xuetian Weng

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