Bug 119367

Summary: strange behavior of "edit entry" in dialog "advanced permissions"
Product: [Unmaintained] kio Reporter: urwald <urwald>
Component: kfileAssignee: Till Adam <adam>
Status: RESOLVED FIXED    
Severity: normal CC: adam
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description urwald 2006-01-02 10:45:17 UTC 
Version:            (using KDE KDE 3.5.0)
OS:                Linux

When working with ACLs with konqueror, it's possible to select the "owner" entry, then choose "Edit entry..." and change it's type for example to named group.

Then, there is no longer an owner entry visible. You must close and reopen this dialog to change the owner permissions. The permissions for owner are then ---. This behavior is confusing. The button "Edit Entry..." should be disabled for owner, owning group and others, who never can't be deleted.

The mask entry can also be changed this way - with the result that you see after closing and reopening the dialog that he he kept it's rights. This makes the confusion perfect. The button "Edit Entry..." should be disabled for the make entry also.

By the way: doing these changes, the dialog crashing quite often when clicking "ok".
Comment 1 Till Adam 2006-01-09 13:46:43 UTC 
Hm, again, the crash is fixed, I believe. Maybe making it impossible to change the type of the default entries is better, you're right. I'll look into this.
Comment 2 Till Adam 2006-01-22 20:32:46 UTC 
SVN commit 501392 by tilladam:

Make it impossible to change the type of the standard entries, make the
behavior of delete more consistant.

BUG: 119367
BUG: 119369


 M  +45 -11    kacleditwidget.cpp  
 M  +6 -0      kacleditwidget_p.h  


--- branches/KDE/3.5/kdelibs/kio/kfile/kacleditwidget.cpp #501391:501392
@@ -91,15 +91,19 @@
 
 void KACLEditWidget::slotUpdateButtons()
 {
-    int selectedItemsCount = 0;
+    bool atLeastOneIsNotDeletable = false;
+    bool atLeastOneIsNotAllowedToChangeType = false;
+    int selectedCount = 0;
     QListViewItemIterator it( m_listView, QListViewItemIterator::Selected );
-    while ( it.current() ) {
-        ++it;
-        if ( ++selectedItemsCount > 1 )
-            break;
+    while ( KACLListViewItem *item = dynamic_cast<KACLListViewItem*>( it.current() ) ) {
+        ++it; ++selectedCount;
+        if ( !item->isDeletable() )
+            atLeastOneIsNotDeletable = true;
+        if ( !item->isAllowedToChangeType() )
+            atLeastOneIsNotAllowedToChangeType = true;
     }
-    m_EditBtn->setEnabled( selectedItemsCount == 1 );
-    m_DelBtn->setEnabled( selectedItemsCount > 0 );
+    m_EditBtn->setEnabled( selectedCount && !atLeastOneIsNotAllowedToChangeType );
+    m_DelBtn->setEnabled( selectedCount && !atLeastOneIsNotDeletable );
 }
 
 KACL KACLEditWidget::getACL() const
@@ -331,7 +335,29 @@
     setText( 5, strEffective );
 }
 
+bool KACLListViewItem::isDeletable() const
+{
+    bool isMaskAndDeletable = false;
+    if (type == KACLListView::Mask ) {
+        if ( !isDefault &&  m_pACLListView->maskCanBeDeleted() )
+            isMaskAndDeletable = true;
+        else if ( isDefault &&  m_pACLListView->defaultMaskCanBeDeleted() )
+            isMaskAndDeletable = true;
+    }
+    return type != KACLListView::User &&
+           type != KACLListView::Group &&
+           type != KACLListView::Others &&
+           ( type != KACLListView::Mask || isMaskAndDeletable );
+}
 
+bool KACLListViewItem::isAllowedToChangeType() const
+{
+    return type != KACLListView::User &&
+           type != KACLListView::Group &&
+           type != KACLListView::Others &&
+           type != KACLListView::Mask;
+}
+
 void KACLListViewItem::togglePerm( acl_perm_t perm )
 {
     value ^= perm; // Toggle the perm
@@ -976,8 +1002,6 @@
 
 void KACLListView::slotRemoveEntry()
 {
-    bool needsMask = findItemByType( NamedUser ) || findItemByType( NamedGroup );
-    bool needsDefaultMask = findDefaultItemByType( NamedUser ) || findDefaultItemByType( NamedGroup );
     QListViewItemIterator it( this, QListViewItemIterator::Selected );
     while ( it.current() ) {
         KACLListViewItem *item = static_cast<KACLListViewItem*>( it.current() );
@@ -987,11 +1011,11 @@
          * removed, or don't remove it, but reset it. That is allowed. */
         if ( item->type == Mask ) {
             bool itemWasDefault = item->isDefault;
-            if ( !itemWasDefault && !needsMask ) {
+            if ( !itemWasDefault && maskCanBeDeleted() ) {
                 m_hasMask= false;
                 m_mask = 0;
                 delete item;
-            } else if ( itemWasDefault && !needsDefaultMask ) {
+            } else if ( itemWasDefault && defaultMaskCanBeDeleted() ) {
                 delete item;
             } else {
                 item->value = 0;
@@ -1014,6 +1038,16 @@
     }
 }
 
+bool KACLListView::maskCanBeDeleted() const
+{
+   return !findItemByType( NamedUser ) && !findItemByType( NamedGroup );
+}
+
+bool KACLListView::defaultMaskCanBeDeleted() const
+{
+    return !findDefaultItemByType( NamedUser ) && !findDefaultItemByType( NamedGroup );
+}
+
 #include "kacleditwidget.moc"
 #include "kacleditwidget_p.moc"
 #endif
--- branches/KDE/3.5/kdelibs/kio/kfile/kacleditwidget_p.h #501391:501392
@@ -78,6 +78,9 @@
     acl_perm_t maskPartialPermissions() const;
     void setMaskPartialPermissions( acl_perm_t maskPerms );
 
+    bool maskCanBeDeleted() const;
+    bool defaultMaskCanBeDeleted() const;
+
     const KACLListViewItem* findDefaultItemByType( EntryType type ) const;
     const KACLListViewItem* findItemByType( EntryType type,
                                             bool defaults = false ) const;
@@ -171,6 +174,9 @@
 
     void calcEffectiveRights();
 
+    bool isDeletable() const;
+    bool isAllowedToChangeType() const;
+
     void togglePerm( acl_perm_t perm );
 
     virtual void paintCell( QPainter *p, const QColorGroup &cg,