Review Request: Fix Open Picture.... menu behavior in Picture Frame applet

Shantanu Tushar Jha jhahoneyk at gmail.com
Mon Jul 6 14:10:20 CEST 2009


On Fri, Jul 3, 2009 at 7:11 PM, Sebastian Kügler<sebas at kde.org> wrote:
> On Friday 03 July 2009 07:06:17 Shantanu Tushar Jha wrote:
>> bool invalidPicture = (m_currentUrl.path() == "Default") &&
>> (m_mySlideShow->currentUrl() == "Default"); if
>> (hasAuthorization("LaunchApp") && ! (m_menuPresent || m_potd ||
>> invalidPicture)) {
>
> I can't see you patch, but this line smells a lot like you don't make a
> difference between remote URL and path. (path() vs url(), basically) Now the
> Picture Frame on *your* disk probably doesn't even support remote URLs, mine
> does however. In general, it's a good idea to not assume all URLs one deals
> with are local. In this case, I'd have to test this specific patch (probably
> won't be hard to fix, if needed at all).
>

So, I should change m_currentUrl.path() to m_currentUrl.url() ?

> I should really get this stuff merged, I'm talking more about it than the
> amount of actual code I put in. :)
> --
> sebas
>
>  http://www.kde.org | http://vizZzion.org |  GPG Key ID: 9119 0EF9
>
> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel at kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
>
>

Also, at times the Open Picture.. option is *still* getting added two
times. Even now I'm searching for the problem, but not any success
till now. I had removed it from init() but still its gettting called
two times. Any idea?

-- 
Shantanu Tushar    (UTC +0530)
http://www.shantanutushar.com


More information about the Plasma-devel mailing list