While the new "delete %nodetype content" permission gives the administrator more granular control, it throws up a new problem:

The delete button is shown only on the edit page, so a user with the permission to delete but not to edit some kind of nodetype, can't do either. Except he knows that he has to append "/delete" to the path!
Of course the question is allowed, whether it makes any sense to grant someone delete but not edit permission. But it is possible.
And if it is possible it should be handled transparently.

Imagine also the (a little far-fetched) case that noone notices that some group has been accidentally granted delete rights. Except for that one person who knows the trick and uses it to make harm. IMHO there shouldn't be any tricks on the user side.

So what to do?
I see the following possibilities:

  1. make 'delete xyz' some kind of sub-permission of 'edit xyz'.
  2. add a 'delete' tab and remove the 'delete' button from the edit form.
  3. add a 'delete' tab if and only if the user has no editing permission and thus no 'edit' tab.
  4. leave it as is.

While 1. would be the most secure solution, we'd need a completely new role permission interface, which is not possible anymore.

Solution 2. has two advantages: -> For someone who wants to delete a node, it takes one page view, one large scroll and one click less. -> For someone who wants to edit a node, it is one button less at the end of the edit form, therefore less confusion and less risk of deleting instead of submitting.

Solution 3. wouldn't change the UI in 99,9% of the cases. But those conditional UI changes make the system less transparent.

Solution 4. would mean: set this issue to "won't fix", and there we go. Don't think we should do that.

For now I like solution 2 best. In case we'd get a hierarchical permission system in D7 we can think again.

Comments

catch’s picture

1. is D7
3. is a nono - fine to add things or remove them, but no change locations if people change roles.

I'd be fine with 2 and 4. I quite like the idea of not having "delete" right next to Preview and Save, it's not a good place for it. So +1 for the tab, if we want to fix this at all.

pancho’s picture

StatusFileSize
new1.74 KB

Okay, for solution 2 - which I prefer as well - I hereby include a patch against HEAD.

Please note, that on the confirmation page now the tabs are shown, which is not good and has to be corrected. I'm sure, someone else knows how to do that, though...

Feel free to test if everything else works fine...

pancho’s picture

Status: Active » Needs review
David_Rothstein’s picture

Title: delete permission » Roles with delete permission don't always have a delete button
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new1.78 KB

Patch no longer applies, so I rerolled and tested. Everything seems to work fine. I actually think it's OK for the tabs to show on the confirmation page... just my opinion, though. Anyway, I'm setting to RTBC -- it's probably good to get this considered before the final 6.0 release since it involves a change in the user interface.

(P.S. There are plenty of use cases for "delete but not edit": Sometimes you might want to allow people to delete obsolete or inappropriate content from the site but not give them the ability to edit old posts and make it look like people wrote things they didn't actually write.)

gábor hojtsy’s picture

Version: 6.x-dev » 7.x-dev

There is absolutely no way we are making that button a tab so late in Drupal 6. Moved to 7.

dries’s picture

I'm not sure. The tab is very awkward for me.

Can people with only the delete permission, delete nodes using the content administration page? If so, that would suffice IMO.

catch’s picture

Status: Reviewed & tested by the community » Needs work

Personally I'd like to see delete (and maybe edit) move to hook_link so they show at the bottom of the node. I'm increasingly less convinced that tabs make sense for this. Think how much nicer it is to delete a comment than it is a node - much less clicks.

David_Rothstein’s picture

Yeah, with three months hindsight, I agree that having it as a tab looks a bit awkward ;)

@catch, that sounds like a GREAT idea... Big +1 on having "delete" appear in the links! Cause, yeah, the time you might want to delete something is usually right after you've read it, so having the link right there at the bottom makes a lot of sense. It's also a very extensible idea -- I can definitely imagine putting an "unpublish" link there also, although that would be the subject for another patch.

I would vote for leaving "edit" as a tab, though. Sometimes you want to edit a node multiple times or without having read through it, so it's nice to have it as a tab right there at the top of the page. (Imagine a node that's 5000 words long or something like that... if you had to scroll down to the bottom of the page every time you wanted to edit it, I think that would get very annoying very fast.)

David_Rothstein’s picture

On the other hand, I guess the counterargument would be that making it too easy and convenient to delete things is not necessarily a good idea, for obvious reasons.....

David_Rothstein’s picture

And one more thought (somewhat obvious, perhaps): I think "edit" should definitely stay as a tab, but maybe it could also appear in hook_link...? A little duplication isn't always a bad thing.

It might look a little weird for a very short node -- but for a very long node, it would be quite useful to have the edit link at both the top and bottom.

David_Rothstein’s picture

Status: Needs work » Needs review
StatusFileSize
new2.35 KB

Here is a quick patch that implements @catch's idea of moving "delete" to the links section. Something feels a little wrong about it right now, but it's a start.

Can people with only the delete permission, delete nodes using the content administration page?

I looked into this a bit, and it seems like it would be possible to implement although it might require a fair amount of reworking of the permissions for that page... (right now everything there is just under "administer nodes")

catch’s picture

Not tried the patch yet but thanks for taking it on. I'd also like to see publish/unpublish and others move to hook_link (this partially inspired by the nice ajax 'add to/remove from queue' links in nodequeue) - it helps to give us much more granular node permissions and might help to declutter both tabs and the node form - and it makes things a lot faster when you just want to set a flag. On the other hand, we could end up with lot of links - wondering if we need an admin flag in hook_link to drop things into a second unordered list. Duplicating edit in both places seems fine to me as well.

In terms of making things easy to delete - not sure if that's such an issue. At the moment we have the Delete button right next to the Save button - which isn't ideal either.

macgirvin’s picture

Just a quick Q: - I'm a bit unclear of the need for the check of $teaser in the patch (#11). I'm not seeing other places in core which pass this variable to node_link(), so I'll concede ignorance as to why a check against it is needed. Just trying to verify that we aren't hiding the delete functionality unnecessarily at times (under what conditions is $teaser going to be 1 [vs. FALSE], And does this mean that the delete function should not be visible in this circumstance?).

Just guessing this is set when we're viewing the teaser as opposed to the whole hog... but I don't believe that fact alone should prohibit node deletion if one has permission to remove it, and if true, potentially contradicts the stated purpose of the patch, to wit: Roles with delete permission don't always have a delete button.

David_Rothstein’s picture

@macgirvin, yup, exactly, checking if (!$teaser) has the effect of only showing the delete link on the full node view. People with delete permissions will always get a delete link at node/xxxxxx (so the bug is fixed either way), but you may be right that it's simpler and clearer to show the link always. And modules could override this if they want to. Thoughts?

@catch, it sounds like you're on to something there with splitting out the "admin" links separately. (I wonder if even a more general solution makes sense; instead of an admin flag, how about letting modules define their own categories in hook_link and then grouping the links by those?) It could be for another issue, but it's definitely one of things that causes this patch to feel a little wrong.... having "Delete" sandwiched between "Add new comment" and "Printer-friendly version", for example, is definitely quite confusing.

Another thing that I don't quite like about the patch right now is that I used "Delete" for the link text (and "Delete this post" for the mouse-over title).... I think it's important to be specific here and be very clear about what is being deleted, and I'm not sure the above does it. We really mean something like "Delete this node" or "Delete the entire node", but figuring out how to write things like that without using the word "node" is not something I'm skilled at ;) Any ideas? (With the current-labeled "Delete" button, a colleague of mine at work was once trying to delete a file attachment but instead wound up clicking on "Delete" and blowing through the confirmation form without even realizing they had just deleted the entire node...... so it's definitely something to think about.)

catch’s picture

@David: added a new issue for hook_link here: http://drupal.org/node/255686

figuring out how to write things like that without using the word "node" is not something I'm skilled at ;) Any ideas?

Yes, bring back node!

I'd prefer links on the full node view only, but I do this in themes anyway, so maybe it's best to stick to whatever's happening everywhere else. I guess if there's a spam post in a teaser view it's also one less click.

cburschka’s picture

Just for consistency, I believe that the "Edit" and "Delete" links should look the same way on the node view. I don't see what is awkward about it. A "Delete" tab is used on other systems, for example Wikipedia.

The reason comments have a delete link instead of a tab is because they don't have tabs. Their edit button is a link, so their delete button is also one. Nodes have an edit tab...

Status: Needs review » Needs work

The last submitted patch failed testing.

lilou’s picture

Status: Needs work » Needs review
catch’s picture

Status: Needs review » Needs work

Arancaytar's right - we should just put the delete into it's own tab - it solves the permissions issue, and reduces a bit of clutter on the node form.

fwiw, I think it'd be interesting to consider putting comment administration in a tab (would have to be js for comment listings) - would result in less people clicking edit when they mean reply/quote (/me raises hand) at least.

pancho’s picture

Title: Roles with delete permission don't always have a delete button » Move delete button to a more appropriate place
Category: bug » task
Status: Needs work » Needs review
StatusFileSize
new577 bytes
new1.76 KB

The main point of the original issue (users with delete, but w/o edit perm) is a won't-fix per Dries' argument - yes the content administration page is enough.
So this is still about a usability aspect: having the 'save' and 'delete' buttons side by side is no good idea, as it is too easily confused - at least one needs to figure out every single time what the buttons actually do.

Here's a rerolled version. I'm back to the tab idea - the delete link didn't convince me in the end.

But there is also another alternative: move the 'delete' button to the right, so it is set apart a bit...

alexanderpas’s picture

'save' and 'delete' next to eachother is horrible usability

I think a tab (LOCAL_TASK) is the best place for the delete button.

if a theme wants to put it for example in a block, as catch suggested, this is still possible.

catch’s picture

Component: node system » usability

Tab seems like our best option to me. Moving this to the usability queue.

Bojhan’s picture

Please supply screenshots,with annotations

David_Rothstein’s picture

Category: task » bug
StatusFileSize
new53.12 KB
new55.13 KB
new48.67 KB

The main point of the original issue (users with delete, but w/o edit perm) is a won't-fix per Dries' argument - yes the content administration page is enough.

No, it's not, because users with the "delete any"/"delete own" permissions do not have access to that page. (You need "administer nodes" in order to access it; #301902: Allow more users to see the node admin page may change that, but it's not in yet.) So, this is definitely still a bug.

As for moving the delete button to a tab, I've attached screenshots of what this looks like. It certainly looks pretty good in the "View" and "Edit" screenshots, but when "Delete" is chosen, it looks kind of weird. Some of this may be specific to the Garland theme, but I think part of the problem is that the page title changes. This pushes the links to the right, which makes it awkward if you are clicking between them.

One possible option: Maybe leave the page title the same (so it's consistent between all three), and move the confirmation message down underneath it?

catch’s picture

Leaving the title of the node sounds good, then the message would be closer to the Delete button.

alexanderpas’s picture

looking very good as a tab :D

+1 for leaving the page title the same (so it's consistent between all three), and moving the confirmation message down underneath it.

alexanderpas’s picture

Component: usability » node system
Issue tags: +Quick fix, +Usability

Status: Needs review » Needs work

The last submitted patch failed testing.

ultimateboy’s picture

There is a patch which makes delete as a local menu task. It is passing tests #400510: Make delete node a local menu task.

Lets consolidate these issues.

michaelfavia’s picture

StatusFileSize
new71.24 KB
new75.02 KB
new171.94 KB
new5.39 KB

'Im not sure i follow why "Delete" looks perverse up as a local menu task (LMT) while View, Edit, Track, Revisions all enjoy the visibility and ease of use. I'm glad consensus has formed around moving delete to a LMT and I've supplied a patch that does just that against UNSTABLE-5.

This patch

  • Enforces consistent use of plain $node->title as the pages title under View, Edit, Delete, Track, and Revisions
  • Adds the previous confirmation messages that were prepended to the title and made it VERY long back to the description argument of confirm_form().
  • Removes the Delete Button and associated form processing logic from the edit form.

Outstanding issues

  • The $description passed to confirm form needs to be themed and prettied up for display but i was sure the most approved way to pass html into such a form. This form might needs a little rework as a result which im happy to do if there is a suggestion on what road to take.
  • Local Menu Tasks dont show up on the delete confirmation form for revisions even though the path resides below the path of the parent LMT. This has always been the case but id like to fix it. Its probably best reserved for another issue but id appreciate help/advice.
  • Please understand that i know this needs theming work on the confirmation pages but that should also be another issue

Argument for the changes above

Hiding delete under edit makes it extra and unnecessary work to delete a node form the viewing of it. It also makes it impossible for people without edit permissions on the node to delete it. Users without administer nodes permission dont see the content admin page and even if they did this still provides value and makes sense (there is no argument for moving edit to content admin only because you can do that there too). Not everyone uses content admin as their primary organizational interface.

Putting it in links defeats the purpose of having structured and weighted Local Menu Tasks in the firstplace. Under such a system why arent view and edit links too? Because you "want to edit/delete something after you read it" is a weak argument in my opinion. There are just as many times that you came to the node to do just that without needing to read it. Especially if you wrote it. Then you are just wasting time again scrolling.

If local menu tasks (aka menu tabs) are broken then lets do what we can to fix them. They have a strong root in providing context for current page view and alternative actions you can perform on that content (be it a node or a content type or a input frormat).

catch’s picture

We just, just introduced 'Edit @type' about 2-3 weeks ago - I don't see any reason for this patch to remove it.

I like delete as a tab though.

michaelfavia’s picture

Title: Move delete button to a more appropriate place » Move delete nodebutton on edit form to a local menu task
Status: Needs work » Needs review

changed verbiage for to developer lingo and marked for review

Status: Needs review » Needs work

The last submitted patch failed testing.

michaelfavia’s picture

StatusFileSize
new5.2 KB

Rerolled patch so address catch's valid issue about no @type on the edit page. Opened #401496: Move node type from title of edit page into the form to solve this issue and ill add a patch after the weekend. No screenshot needed last one applies just add "Edit @type " before the title of the page.

catch’s picture

Status: Needs work » Needs review

Removes a bunch of lines of code, fixes a permissions issue, if applied to users it might prevent g.d.o site admins from deleting their own accounts late at night. Marking CNR for the testbot, rtbc when it's all green.

michaelfavia’s picture

Status: Needs review » Reviewed & tested by the community

As referenced by catch above. Previous testing error was a HEAD issue I think.

dries’s picture

If we change this, we have to change it everywhere and make it consistent across Drupal. It doesn't feel like there is solid consensus about whether to have this as a tab or not. This is the kind of feature that could do with some usability testing.

michaelfavia’s picture

@Dries - I already promised catch and yoroy that id do exactly that on monday. This weekend is a little busy with SXSW stuff. I'll submit issues and patches for each module if youd like or i can roll them into additional 1 issue. However youd like to handle it.

dave reid’s picture

+1 as a tab. It's one of the main reasons I created the admin_links module. All the other node operations are tabs, but delete is magically hidden as a button on the edit form.

rickvug’s picture

StatusFileSize
new10.02 KB
new74.12 KB

A user's expectation is that tabs reveal more UI options. Having an action such as delete as a tab violates this rule. It doesn't look or feel right to me. I agree with Dries on this one: there is no obvious answer and any potential solution should be tested.

Besides the "tab as an action" complaint above, making delete a tab gives the option too much prominence. As I see it, the current usability issue is that delete is given the same amount of prominence as save or preview. The placement is directly beside two non destructive tasks, increasing the chance of an accidental click. Moving delete to a tab would move the same prominence issue into another place in the UI. I think that the ultimate solution will involve making delete somehow different than the other options given to the user.

I've attached two screenshots from 37signals' Basecamp and Highrise to show how they have approached similar issues. In Highrise they de-emphasized delete for companies and individuals by putting the option in its own sidebar area. For messages they haven't done this, presumably because they are not as critical as contact information. In Basecamp and elsewhere they've also interestingly put "cancel" beside save and preview but de-emphasized the option. Maybe something like this could work for delete as well?

rickvug’s picture

StatusFileSize
new173.87 KB

I've attached another screenshot, this time from Wordpress. Wordpress places delete near "Update Post" but it is separate and red, indicating that it is a fundamentally different activity. IMO, something similar could work well for Drupal.

alexanderpas’s picture

Priority: Normal » Critical

A user's expectation is that tabs reveal more UI options. Having an action such as delete as a tab violates this rule. It doesn't look or feel right to me. I agree with Dries on this one: there is no obvious answer and any potential solution should be tested.

actually... it does reveal something, it reveals the actual option to delete the item, see phpMyAdmin for another example!

Besides the "tab as an action" complaint above, making delete a tab gives the option too much prominence. As I see it, the current usability issue is that delete is given the same amount of prominence as save or preview. The placement is directly beside two non destructive tasks, increasing the chance of an accidental click. Moving delete to a tab would move the same prominence issue into another place in the UI. I think that the ultimate solution will involve making delete somehow different than the other options given to the user.

actually... clicking the delete button instead of save is more destructive.

scenario 1:
user goes to node/24
user clicks edit (node/24/edit)
user makes some edits.
user clicks delete button instead of save.
user notices error.
user backs out using cancel links
user arrives on node/24
DATA LOSS!!!

scenario 2:
user goes to node/24
user clicks edit (node/24/edit)
user makes some edits.
user clicks delete button instead of save.
user notices error.
user backs out using back button
user arrives on node/24/edit
user doesn't know if changes were saved
(no they weren't!)
DATA LOSS!!!

scenario 3:
user goes to node/24
user clicks delete instead of edit (node/24/delete)
user notices error.
user backs out using cancel button,
user arrives on node/24
user clicks edit (node/24/edit)
user makes some edits.
user clicks save.
NO CONFUSION!!!

scenario 4:
user goes to node/24
user clicks delete instead of edit (node/24/delete)
user notices error.
user backs out using back button,
user arrives on node/24
user clicks edit (node/24/edit)
user makes some edits.
user clicks save.
NO CONFUSION!!!

scenario 5:
user goes to node/24
user clicks delete instead of edit (node/24/delete)
user notices error.
user backs out using edit link,
user arrives on node/24/edit
user makes some edits.
user clicks save.
NO CONFUSION!!!

rickvug’s picture

"actually... it does reveal something, it reveals the actual option to delete the item."

It is obvious that a delete tab would reveal something. However, what it is actually being revealed is what is now a confirmation message. I would rather not have Drupal start putting confirmation messages under their own tabs. I think this is especially important if/when the modal patch gets in.

The data loss scenarios 1 + 2 are true. My hope would be that we make the delete option distinct enough that it will be clicked accidentally very rarely. That is what the Wordpress, Basecamp and Highrise screenshots illustrate. To point to the modal patch again, I think the "back button" or "cancel" data loss scenarios would be greatly improved if confirmation dialogs didn't require a page refresh. Problem solved?

At the very least, if delete does become another option beside delete I'm hoping that we find some way to differentiate it. Thank you for the healthy debate about this.

rickvug’s picture

I decided to take a look at where Joomla and Blogger place their delete links. Interestingly, neither had delete on the edit page or as a tab. Both only allowed deletion from the content listing screen (aka. admin/content/node). I don't think this is the solution for Drupal but it is food for thought.

michaelfavia’s picture

@rickvug: "A user's expectation is that tabs reveal more UI options. Having an action such as delete as a tab violates this rule." Can you please explain what you mean by this? How is delete any different from edit in this regard?

It is THIS (and only this) users understanding that LMTs (aka tabs) present alternative actions on the menu item that is their direct parent (settings in admin {taxonomy, content types, etc), content action items in nodes. I just dont see the distinction that you are talking about and neither do my users. I'm all for usability testing and im sure you have a valid point but i just cant make sense of it from my perspective. -mf

michaelfavia’s picture

Ok so i tackled the unification across core that Dries mentions in a personal blog post (link at bottom of comment). I'll create a bunch of dependent issues on each module as i push through them and and link to them in the comments of the meta issue i opened (link at bottom) for my own organization while i work through all of the details. I have to perform a little usability testing (wit half a dozen subjects or so) on the 2 major use cases (node and admin settings). stompeers has already offered to help with the patches. Good day.

Blog post describing: http://favias.org/post/good-riddance-delete-edit-form
Meta Issue for the remaining areas of core: #402760: Regression: Turn "delete tabs" back into buttons

webchick’s picture

Status: Reviewed & tested by the community » Needs review

For now I'm setting to "needs review" still since we're still lacking some formal data to back up this change. I spoke with Michael on IRC the other day about ways to gather some lo-fi feedback from people just to confirm this is a good direction before we head down this path.

alexanderpas’s picture

I decided to take a look at where Joomla and Blogger place their delete links. Interestingly, neither had delete on the edit page or as a tab.

I decided to take a look at where MediaWiki and phpMyAdmin place their delete links. Interestingly, both had delete as a tab. (sorry, coudn't resist)

Both only allowed deletion from the content listing screen (aka. admin/content/node). I don't think this is the solution for Drupal but it is food for thought.

that's because both still work according the backend/frontend paradigm, while Drupal clearly works according the inline admin paradigm (just like MediaWiki and phpMyAdmin).

webchick’s picture

FWIW, I would never in a million years call either MediaWiki or PHPMyAdmin applications that have ground-breaking usability. ;) That is probably hurting your cause more than it is helping.

michaelfavia’s picture

Usability testing with 4 new users to drupal at SXSWi indicates that delete as a LMT is much more intuitive than hiding it under "Edit. Interestingly enough Delete was found under edit after searching for it (something i was surprised to see). The content administration screen wasnt ever used. This is probably because subjects were asked to create new content and delete it from the default node listing page and werent asked to configure the site. This is probably a common use case for content editors and creators.

michaelfavia’s picture

An additional testing of 2 new drupal users and 1 existing user with basic content editing experience agreed that this is a better user interaction. The tests were presented in random order to each subject incase the order biased their results. The subjects were unquestioning in their preference. The inline administration makes extension of functionality easier and applies to a key UX tenent of "putting the controls where the content is" (aka edit in place, etc).

@webchick i leave this issue here for you unless youd like me to do anything additional. Once/if this issue is committed ill attack the other areas as Dries requests.

catch’s picture

Status: Needs review » Needs work

That sounds like really good feedback in testing.

I took a final pass through the code, everything looks great except I think we're missing a check_plain in the confirm_form() changes:

+  return confirm_form(
+    $form,
+    $node_revision->title,
+    'node/' . $node_revision-

confirm_form, uses PASS_THROUGH in drupal_set_title() for $node_revision->title - so this'd be XSS.

So needs work for that change, otherwise I think we're good.

michaelfavia’s picture

Status: Needs work » Needs review
StatusFileSize
new5.24 KB

Addressed check plain in ALL confirm_forms. Was previously passing through t() which is why i missed it. Please review and rtbc if ready.

catch’s picture

Status: Needs review » Reviewed & tested by the community

Yep I had to look at least three times before I caught it and it's a 5k patch.

RTBC.

webchick’s picture

Just to check in, I'm fine committing this patch (and thanks, Michael for the user research!) However, since Dries voiced doubts earlier in the issue, I'd like to discuss it with him first. Should be able to do that on Wednesday.

webchick’s picture

Issue tags: -Quick fix

Oh, and this is anything but a "Quick fix" ;)

michaelfavia’s picture

@ webchick sounds good to me. As soon as this first one goes in ill start pushing the remaining ones in as well. They are enumerated on this page: http://favias.org/post/good-riddance-delete-edit-form . It should take me about a week to get them all in and updated. I'll group them by module if youd like to keep patch sizes down and reviewable. Please let me know in my comments on my site or here if you disagree with any of the items or the general direction for these. The direction is summed up pretty well in my "interaction modalities" list, but id be happy to revise or explain anything determined by you and dries.

dries’s picture

michaelfavia, a more detailed report from your usability testing would be useful. We don't know how the tests were performed, what tasks were given, what questions were asked, how did you concluded one was better than the other?

rickvug’s picture

#45 @michaelfavia "A user's expectation is that tabs reveal more UI options. Having an action such as delete as a tab violates this rule." Can you please explain what you mean by this? How is delete any different from edit in this regard?"

Currently confirmation of content delation takes you to another page. However, I suspect (and hope) that this will not be the case by the time D7 launches. See #374646: Popbox (Popups Lite): Adding Modal Dialogs to Confirmations where Dries mentions his support for pop-ups in core. If this patch lands, confirmation of deletion should be adapted to use modal dialogs like most (all?) other modern web applications. In this scenario, this patch would result in a tab that is no longer acting as a tab should. If the pop-ups patch does not get in, having delete as a tab could make sense.

As for my earlier concerns about placement, I could go either way. My gut feeling is that this gives too much emphasis to an option that most users should not be clicking. However, the correct placement of delete links really depends on the application and the likelihood that users will want to delete content (and the ramifications of doing so). What is needed is a fallback that is best in the majority of use cases.

If user testing shows less confusion about deletion, perhaps this should go in. However, I'd still like serious thought given to the tab scenario discussed above. If the answer is to have delete outside of edit, perhaps the link should be styled differently than a tab. I'm not sure what this would look like but I'd like to throw the option out there for consideration.

dave reid’s picture

Status: Reviewed & tested by the community » Needs work

Need to fix one small coding standard:

+    'weight' => 2,
+    'type' => MENU_LOCAL_TASK);
Should be:
+    'weight' => 2,
+    'type' => MENU_LOCAL_TASK,
+  );
michaelfavia’s picture

@dries: Here is the setup and script i followed.

Setup

* Create 2 demo site installs with devel generated article content content (one for pre and post patch)
* Create a user/role with permissions to create, edit and delete ALL articles.

Script

The script was designed to make sur ehtat the user had created and edited a post at least once in drupal before asking them to delete it. This was to make sure they had at least seen the option to delete the content in both pre and post patch versions before the actual task of deleting the object began. Each participant was asked to think aloud as they navigated for feedback.

* Randomly alternate between pre and post patch versions as the first instance presented to the user (to be objective)
* Present new user with the default front page and show them a specific article
* Explain that they can create new content like they might on facebook or the like.
* Asked them to create their own piece of content by saying "Please make a new post" (Didnt want to prompt the "create content" menu item.)
* asked them to edit that article and add another sentence.
* Ask them to delete the first example article on the home page that was specifically referred to in the introduction of the site. (Wanted to make sure they disassociated it with the article they created because the delete LMT would be right at the top of this page in the post patch version).
* Rinse wash repeat with the other version.

Summarized results

These results are subjective and i didnt video record the tests because they were really informal. I really like the UX PIP setup though and i think ill use that on my webcam enabled laptop for the next ones so people can just watch.
* Participants had very little difficulty creating new content.
* Participants DID take a moment to find the Edit LMT when asked to edit the article.
* Only one participant (not the experienced one) noticed the Delete button on the edit form and was able to navigate back to it without trouble all others stumbled onto it after searching the page hesitantly and looking for available options
* Nobody used the content management page.
* All participants found the delete LMT with less difficulty than they found the edit LMT and said so when asked which method for deleting the content they preferred.

My Conclusions

The uncluttered and sterile nature of the site and menu might have aided the easy location of the "Create content" menu item, because ive had a number of clients in the past have to have this explained on their finished sites. Add in a few more contrib modules with menu structures and the barrier goes up with each. I also think the general visibility of the LMTs was also aided by the lack of competing content in the form of advertising and meta content blocks. On a normally themed site the theming of the location and function of these components is almost always a sticking point for my users as well (and something i pay special attention to).

That said, because the LMTs stood out (relatively), and once the relationship of the LMTs to the piece of content is understood it DID seem to function as a SAFE place to look for things to do to an article for these users (Edit Delete, etc). I didnt test it but i would imagine this might expand to Revisions and Track as well as contrib modules like Voting, etc. I think the primary sticking point here though is the need to better emphasize the presence or the functionality of this user interaction (LMTs). I hope that there might be a solution for this at the theme level because the idea of implementing these options as nodelinks makes my skin crawl a little bit.

It is worth noting that none of these users knew what a CMS was or had any real experience with one (they just wanted to eat a taco in peace). So take their input for what it is worth. If youd like me to revise and ask one or two more people im happy to, if you'd like to conduct the test yourself id be even happier to see the results.

cburschka’s picture

Status: Needs work » Reviewed & tested by the community
StatusFileSize
new5.55 KB

Re #60 by David:

Note that this is not the only place where the same stylistic error occurs in node.module. This patch fixes the one it touches, but leaves the others alone for a follow-up issue. Fixing style errrors in passing is good, but general clean-up should probably be in a separate patch. Setting back to RTBC as the style error was the only change since then.

Now that the minor things are out of the way, we just need to decide if the change is going to go through at all. ;)

dave reid’s picture

StatusFileSize
new5.55 KB

@Arancaytar I agree, but for any line of code that a patch changes, those lines should be subject to the coding standards. You still missed the trailing comma in your re-roll so I changed it for you.

michaelfavia’s picture

@rickvug: your statement about " My gut feeling is that this gives too much emphasis to an option that most users should not be clicking." assumes that you knwo what people are doing with nodes and that they represent some type of permanent content. For many applications of drupal such as classifieds or things of a temporal nature like coupons/specials or events, this isn't the case.

IMO the argument here as ive made a few times is that each of these actions (view, edit, delete) maps to the object that they are decorating (a node) and all are first class actions. Couple that with the difficult i mentioned earlier with the permissions not allowing users with appropriate access ANY way to delete the content they should be able to and i think this makes a pretty good case. We prevent accidental data loss with a confirmation page, i think this is a good compromise. -mf

webchick’s picture

Status: Reviewed & tested by the community » Postponed

Hm. I'm really sorry, folks... but I think we need to postpone this. I'm not really comfortable making such a sweeping change when, based on the stuff at http://www.d7ux.org/d7ux-initial-concepts-direction/, we might not even have things like local tasks to begin with.

Additionally, although I appreciate the thoroughness with which Michael documented his research, I'm not sure that it's as conclusive as we'd need to support making such a massive change. Logically, it makes tons of sense for local tasks to be verbs related to what you can do to a thing, but in practice that's not how we actually use local tasks consistently. While I am not normally in favour of holding up real, working patches over stuff that hasn't been invented yet, in this case I really think we need to put this issue on hold for a bit, participate in the work that Mark and Leisa are doing, and then come back and re-evaluate whether this is a good direction for us.

Sorry, Michael. :(

David_Rothstein’s picture

I really think this should be "needs review" rather than "postponed"... It may not be ready to be committed yet, but why should that stop interested people from doing further research or working more on the patch?

Also, as far as I can tell from http://www.d7ux.org/d7ux-initial-concepts-direction/, local tasks are one of the few things that are being kept in that proposal :) In fact, they're essentially proposing to add some new ones -- see around 3:30 in the video. I had actually even sent them some feedback that I thought what they were doing with that was very consistent with other work currently going on in the issue queue, and this issue was the one I had in mind..... so am I wrong?

webchick’s picture

Status: Postponed » Needs review

OK, I'm cool with "needs review". But definitely needs some more discussion/debate before committing. :)

rickvug’s picture

@michaelfavia In my previous comment I stated that I could go either way about having delete under edit. The best placement of delete depends on the context. You are correct that it should be easy to delete temporal content such as coupons or classifieds. I feel that the opposite is true for content such as pages. For the sake of consistency, it may be that delete is made a top level option.

In the case that delete confirmation becomes a modal dialog, what are your thoughts about delete still being a tab? In your opinion, would this be inconsistent with how tabs work in Drupal?

michaelfavia’s picture

@webchick, I'm sorry but this sounds alot like Dries' attempt to wiggle out of drupal_static() last week. You don't have any actual objections to the patch or the issue but cast vague aspersions on its merit and ask for more debate. This issue is almost 2 years old now and i thought we were on the right path. If you have real objections to the work I've done I'd appreciate knowing how I can improve it or what you suggest as a better model as a lot of people agree with this change to fix a BUG in drupal presently (people with delete permissions cant actually delete content). If you need specific people to agree to the change before your comfortable ill be happy to track them down and solicit opinions. Be specific with me and I'll deliver what you ask for. Your response above is just a basic "lets forget about this" and that isn't appreciated.

Not putting it in because someone is in the imagining stage of building a better mousetrap is a poor excuse not to proceed in my opinion. While I appreciate the work that Mark and Leisa are doing their abstract principles and as of yet well grounded ideas are quite a ways off from being implemented and quite a few problems will inevitably emerge (like computed fields for edit in place, or the required scrolling that will be required for an overlay window that attempts to edit a complex node type, etc). It should't prevent progress that is ready now.

What is being hurt by moving forward with REAL progress and code while we wait to see what useful things might emerge from their work? If i need to provide videos of user testing I'm happy to if i need to do something else name it and I'll provide it if its within my capabilities. "Wait and hope" worked for Edmond Dantes but he didn't have a day job. -mf

webchick’s picture

Hm. It wasn't my intention to say "Let's forget about this" -- it was my intention to say "Let's hold off on this until we see how the D7UX work shapes up," which I'm pretty sure is what I said. :D

Now, it's fully possible we come back in a month and commit this patch as-is, but since it affects everyone from the end users visiting the site to contributed module authors to people building Views, then yes, I do want to be more confident that it's the right path before we proceed, because it's a sweeping change.

It would be different if this were something like Vertical Tabs, where there is overwhelming consensus that it is light years beyond what we have, and has been field tested to improve usability (or at least not seriously detract from it). But I don't get that "universal consensus" sense about this patch. I see lots of very strong disagreement (including from Dries), alongside the strong agreement opinions. While we have some testing data, it is narrow in scope and isn't recorded in a way that people less "invested" in this issue could evaluate objectively. Obviously, not everyone will always be happy with every patch that gets committed, but we also don't want to simply commit every patch that happens to contain no whitespace issues, especially when it might conflict directly with the work the larger Drupal community is undertaking along with Mark and Leisa.

In terms of a step forward, I suppose more complete (and recorded in some way) testing data could help, but honestly what I'd prefer to see is people like you who are passionate about fixing this kind of over-arching user interface stuff collaborating with Mark and Leisa on a more holistic approach that will get proper attention -- user testing, iterations back and forth, heated debate, etc. And at the end of the day implement a solution (which might very well include this patch as a sub-issue) to Drupal's user experience woes that we can all be happy with, or at least can point to data and go "Ah-ha. So this is why we're doing this this way."

I'm trying hard here to strike a balance between blazing boldly ahead with radical usability improvements and doing so too early, only to have them ripped out later when the entire user interface changes later on, which is also going to lead to poor morale. Hope you can understand.

catch’s picture

I don't think it's a problem to commit incremental improvements now then rip them out later - we do that all the time with API level changes - fix a bug in an existing API, then completely re-factor it later during the same release cycle, no-one's heart gets broken by that process.

The question for me in these cases is whether this is going to make it harder to do sweeping changes later, IMO this is a sufficiently small change at the code level that it has no effect on that in either direction. The only thing it does which locks us in in any sense is enforce that our CRUD permissions are actually possible to use in core for people who have them - IMO that's a great constraint to have. However if this gets in, it should have a test for a user with 'delete own' and 'delete any', but not 'edit own' - so that we don't introduce regressions.

If we're going to postpone patches until we're in the implementation phase of D7UX, then review them again after that - then they're going to be postponed until Drupal 8 (or hopefully marked duplicate if they get addressed by it) - let's be clear about the timeline here.

If the consensus ends up being that this isn't an incremental improvement (although I don't see another non-sweeping way to fix the permissions issue), let's mark it won't fix. If it's going to make Mark and Leisa's stuff potentially harder to implement, then we can postpone it (but I doubt that), otherwise we might as well postpone all interface-changing issues for the next three months. Do we need to postpone text changes on the Drupal's personality discussion as well?

webchick’s picture

I don't see how in the world you could possibly view this as an incremental improvement.

If we commit this, we need to also commit the same change to block, comment, node, content type, user, taxonomy term, vocabulary, forum, menu, etc. forms to conform to this convention if it's decided here. That is anything but an incremental improvement. That is a sweeping change, and if we're going to do such a thing it needs to be well-considered and tie into other efforts.

I'm not in favour of postponing every UI-affecting patch until later on, but on issues that affect almost every single form without a very clear consensus backed up by hard evidence, well, yes. I think the effort that would need to go into implementing this change throughout would be better spent on the larger over-arching issue of improving Drupal's UX, and hopefully we can come back more confident that it's the right decision within a few months.

Again, this is not won't fix. It's postponed, in my view.

catch’s picture

Status: Needs review » Postponed

Back to postponed then.

Richard Little’s picture

Title: Move delete nodebutton on edit form to a local menu task » Unable to delete Form Components

Unable to delete form components.

Delete option is not visable in the Form Component tab of Web forn (only edit is viewable but I can see clone when I try and move a component). Is the delete button hidden?

No delete option is available if I use edit option either.

Help requested!

Richard

dave reid’s picture

Title: Unable to delete Form Components » Move delete nodebutton on edit form to a local menu task

Richard, please do not hijack the thread. Please file an issue with the webform issue queue: http://drupal.org/project/issues/webform

michaelfavia’s picture

@webchick @dries: is there any interest in me rerolling this?

rickvug’s picture

@michaelfavia - Please see #542658: Move action "tabs" out of local tasks. I think you will be interested. The patch introduces a new space where actions such are delete could live. My main argument against delete becoming a tab is that tabs should only be used for "data views" rather than actions such as delete. If this patch lands, my understanding is that this would no longer be an issue as you'd be using the "action" local task, which would not be styled as a tab.

marcvangend’s picture

#542658 is fixed and closed, but as far as I can see, 'delete' is not a local action yet. If there's anything to do here I'd like to help out. If not, should this be moved to D8?

David_Rothstein’s picture

Title: Move delete nodebutton on edit form to a local menu task » Having delete as a button on the node edit form means certain users don't have access to it when they should
Priority: Critical » Normal
Status: Postponed » Needs work

The original bug reported here still exists... although I believe it's now the case that you can (safely?) give these users the 'access content overview' permission and then they at least then have some way to delete stuff.

The button is still in a weird location, though, and this issue shouldn't be marked postponed. But it isn't a critical bug.

sun’s picture

Contextual links now expose a "Delete" link that directly points to the node delete form.

It's very odd that you don't find such a link on the node page itself. This confused me two times today.

Workaround: Either click "Edit" and then the "Delete" button. Or go back to the front page or content overview page where you came from and use the contextual link or whatever other tool there.

-1 for removing the "Delete" button from the form.

+1 for simply changing MENU_CALLBACK to MENU_LOCAL_TASK.

sun’s picture

Component: node system » node.module
AlexisWilke’s picture

For D6 users, I created a module with that very functionality (and others too) and you can find it here:

http://drupal.org/project/mini

I hope D7 will have that fix in since it makes sense to have. There may be other forms than just the Node form with the problem... Also, on top of my head I cannot think of one.

Having to use the Content list sucks (someone mentioned that). It is very difficult to find the node in that list and the filtering does not help much if most of your posts are say of type Blog.

Sorry for the noise! 8-)

Alexis Wilke

Tor Arne Thune’s picture

Version: 7.x-dev » 8.x-dev

Still a valid issue. Moving to 8.x.

dave reid’s picture

sun’s picture

xjm’s picture

Component: node.module » node system
Issue summary: View changes

(Merging "node system" and "node.module" components for 8.x; disregard.)

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Version: 8.9.x-dev » 7.x-dev
Issue tags: +Bug Smash Initiative

I tested this on a standard install of drupal 7.79-dev and Drupal 8.9.0. In both cases I created a user with the permission 'Article: Delete any content'. For the Drupal 8 site I added a node of type article. The Drupal 7 site had nodes created by devel_generate.

I then logged in as the user with the delete permission. In Drupal 8 there was a delete tab, so I could delete the node even though I could not edit it. For Drupal 7, there was no button to delete the node and no edit option either. I could navigate to node/edit and delete the node.

Since this cannot be reproduced on Drupal 8 but can on Drupal 7, moving to the Drupal 7 issue queue.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.