Review Request 126505: Do not show a warning color before the user even started typing

[David Faure]

> On Jan. 2, 2016, 11:33 a.m., David Faure wrote:
> > Ship It!
> 
> Elvis Angelaccio wrote:
>     Uhmm, the new tests don't pass locally on my system. I'm fairly sure it's because I have kwidgetsaddons 5.17 globally installed from archlinux repos (indeed, the warning color in my local tests is set as soon as something is typed as password, which is the old behavior). Could someone who has kwidgetsaddons installed from git, verify that these new tests pass? (and if they do, feel free to even commit on my behalf).
> 
> David Faure wrote:
>     You're committing changes to kwidgetaddons, how can it be that you're not compiling and testing it?
>     
>     You should be able to run the test without installing kwidgetaddons, just by setting LD_LIBRARY_PATH to point to the dir where the uninstalled lib is.
>     
>     I thought the RPATH handling made this automatic, even... at least it works here:
>     
>     $ objdump -p ./knewpasswordwidgettest | grep PATH
>       RUNPATH              /d/qt/5/kde/qtbase/lib:/d/kde/build/5/frameworks/kwidgetsaddons/src
>     
>     $ unset LD_LIBRARY_PATH # because here it points to the install dir, but if you use distro packages I guess you didn't set it anyway
>     $ ldd ./knewpasswordwidgettest | grep -i widgetsaddons
>             libKF5WidgetsAddons.so.5 => /d/kde/build/5/frameworks/kwidgetsaddons/src/libKF5WidgetsAddons.so.5 (0x00007f3f73a92000)
> 
> Elvis Angelaccio wrote:
>     > $ unset LD_LIBRARY_PATH
>     
>     This did the trick! It turns out I actually had LD_LIBRARY_PATH defined:
>     
>     $ echo $LD_LIBRARY_PATH
>     /home/elvis/GNUstep/Library/Libraries:/usr/lib
>     
>     That's why the tests were using the installed library. Thanks David :)

Yep, there's no good reason to ever have /usr/lib in your LD_LIBRARY_PATH. It will be found anyway since it's the system fallback, but if you set it explicitly then RPATH can never work.


- David


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


On Dec. 27, 2015, 2:40 p.m., Elvis Angelaccio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126505/
> -----------------------------------------------------------
> 
> (Updated Dec. 27, 2015, 2:40 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability, Christoph Feck, and David Faure.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> -------
> 
> As discussed in RR 125619 and 126426, the password verification field (in a KNewPasswordWidget) should not be marked as "wrong" before the user even started typing the verification password.
> 
> The revised approach is the following:
> 
> * The user starts typing something as password, e.g. 1234
> * The user types something else as verification password
> * As soon as the verification is not anymore a prefix of the password (e.g. verification = 122), the warning color is shown.
> * As soon as the verification is a prefix again (e.g. the user deletes the second 2, i.e. verification = 12) the warning color is not shown anymore.
> 
> 
> Diffs
> -----
> 
>   autotests/knewpasswordwidgettest.h 43845128adec01aced4353c9f7986b7977829a2a 
>   autotests/knewpasswordwidgettest.cpp 297b88d5f18b9cd37f0d26d94e56f38870756f20 
>   src/knewpasswordwidget.cpp a1b59454a2c2d7c09ac32acec52d3fffa73f77fc 
> 
> Diff: https://git.reviewboard.kde.org/r/126505/diff/
> 
> 
> Testing
> -------
> 
> Autotests assert what described above. Gif pictures would explain the patch better than 1000 words, but I suck at creating them :(
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160104/5518713d/attachment-0001.html>