--===============5666929942757317636== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Jan. 2, 2013, 3:18 p.m., David Faure wrote: > > kio/kio/kdirlister.cpp, line 1080 > > > > > > I don't understand the if(). Why should the behavior differ dependi= ng on whether we have this url currently shown (itemsInUse) or in the cache= (itemsCached), compared to when we don't know about this URL? > > = > > The if() is wrongly written anyway: if it should be there, then it = should be url.url(), not url.path(), since itemsInUse and itemsCached use U= RLs as keys. I should change that to QUrl in KF5. > > = > > And in case you remove the if, then my old comment meant: in that c= ase, we can simplify the code to remove the if (isDir), and simply do > > = > > Q_FOREACH(const QString& dir, directoriesForCanonicalPath(ur= l.directory())) { > > handleDirDirty(dir); > > } > > = > > in all cases (file or directory). > > Because as you say, it's the parent that needs to be re-listed when= the permissions of anything changes. > > = > > = > > Also, this changes misses a corresponding unittest in kdirlistertes= t.cpp. Ahh... I see what you mean. I should have realized what I was doing is utte= rly unnecessary. I will make the change as you suggested and add a test cas= e for this. In fact, I will add the unittest first to test for the failure. - Dawit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103555/#review24439 ----------------------------------------------------------- On Dec. 29, 2012, 8:31 p.m., Dawit Alemayehu wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103555/ > ----------------------------------------------------------- > = > (Updated Dec. 29, 2012, 8:31 p.m.) > = > = > Review request for kdelibs and David Faure. > = > = > Description > ------- > = > If you open a directory that contains other directories in Konqueror or D= olphin, change the permission of one of these directories from outside, say= the command line, and right click on the same directory to look at the per= mission tab in the properties dialog, you will see that the permission chan= ge has not been updated. This patch addresses that bug. > = > = > This addresses bug 173733. > http://bugs.kde.org/show_bug.cgi?id=3D173733 > = > = > Diffs > ----- > = > kio/kio/kdirlister.cpp ec3d622 = > = > Diff: http://git.reviewboard.kde.org/r/103555/diff/ > = > = > Testing > ------- > = > 1. In konsole, create a test directory within another test directory: > mkdir -p test/test1 > = > 2. Open Dolphin or Konqueror and navigate to the top newly created direct= ory, i.e. test. > = > 3. In konsole, cd into the first test directory: > cd test > = > 4. In konsole, change the permission of 'test1' from konsole. For example, > chmod go-rx > = > 5. In the open Dolphin or Konqueror, right click on "test1", select prope= rties and click on permission tab. > = > 6. Validate whether or not the permission shown in the GUI matches what y= ou get in the command line. > = > = > Thanks, > = > Dawit Alemayehu > = > --===============5666929942757317636== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/103555/

On January 2nd, 2013, 3:18 p.m., David Faur= e wrote:

= = =
kio/kio/kdirlister.cpp (Diff revision 1) 
void KDirListerCache::slotFileDirty( const QString& path )
 

    1080
    
        if (!itemsInUse.contains(url.path()) &&am=
p; !itemsCached.contains(=
url.path())) {

  


 




  
I don'=
;t understand the if(). Why should the behavior differ depending on whether=
 we have this url currently shown (itemsInUse) or in the cache (itemsCached=
), compared to when we don't know about this URL?

The if() is wrongly written anyway: if it should be there, then it should b=
e url.url(), not url.path(), since itemsInUse and itemsCached use URLs as k=
eys. I should change that to QUrl in KF5.

And in case you remove the if, then my old comment meant: in that case, we =
can simplify the code to remove the if (isDir), and simply do

       Q_FOREACH(const QString& dir, directoriesForCanonicalPath(url.di=
rectory())) {
            handleDirDirty(dir);
        }

in all cases (file or directory).
Because as you say, it's the parent that needs to be re-listed when the=
 permissions of anything changes.


Also, this changes misses a corresponding unittest in kdirlistertest.cpp.
 









Ahh... I see what you mean. I should have realized what I was doing =
is utterly unnecessary. I will make the change as you suggested and add a t=
est case for this. In fact, I will add the unittest first to test for the f=
ailure.








- Dawit






On December 29th, 2012, 8:31 p.m., Dawit Alemayehu wrote:









 

  


Review request for kdelibs and David Faure.


By Dawit Alemayehu.




Updated Dec. 29, 2012, 8:31 p.m.








Descripti=
on 

 

 

  
   
If you open a directory that contains other directories in K=
onqueror or Dolphin, change the permission of one of these directories from=
 outside, say the command line, and right click on the same directory to lo=
ok at the permission tab in the properties dialog, you will see that the pe=
rmission change has not been updated. This patch addresses that bug.

  		
 






Testing <=
/h1>


 

  
   
1. In konsole, create a test directory within another test d=
irectory:
     mkdir -p test/test1

2. Open Dolphin or Konqueror and navigate to the top newly created director=
y, i.e. test.
   =

3. In konsole, cd into the first test directory:
     cd test

4. In konsole, change the permission of 'test1' from konsole. For e=
xample,
     chmod go-rx

5. In the open Dolphin or Konqueror, right click on "test1", sele=
ct properties and click on permission tab.

6. Validate whether or not the permission shown in the GUI matches what you=
 get in the command line.

  		
 








 Bugs: 


 173733







Diffs=
 


    

 
  • kio/kio/kdirlister.cpp (ec3d622)





View Diff





  		
 











  

 



--===============5666929942757317636==--