Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
On NodeTypeForm, the following description is incorrect:
Users with the Administer content permission will be able to override these options.
see the screenshot..
At least the "Create new revision" is not related to the Administer content permission.
\Drupal\Core\Entity\ContentEntityForm::addRevisionableFormFields()
$form['revision'] = [
'#type' => 'checkbox',
'#title' => $this->t('Create new revision'),
'#default_value' => $new_revision_default,
'#access' => !$this->entity->isNew() && $this->entity->get($entity_type->getKey('revision'))->access('update'),
'#group' => 'revision_information',
];
This checkbox can be edited by anyone, as Drupal core does not apply any access restriction on this field (with hook_entity_field_access())
Comment | File | Size | Author |
---|---|---|---|
#36 | Screenshot from 2020-02-04 15-22-54.png | 201.01 KB | Rithesh BK |
#16 | 2946177-16-D8.patch | 744 bytes | mohit1604 |
NodeTypeForm.png | 85.92 KB | Pasqualle |
Comments
Comment #2
bhanuprakashnani CreditAttribution: bhanuprakashnani at Google Summer of Code commented@Pasqualle
I have understood that we have to correct some incorrect text. So what is the next step to continue with the bug report? Please assist me.
Comment #3
cilefen CreditAttribution: cilefen as a volunteer commentedCreate a patch: https://www.drupal.org/node/707484
Comment #4
bhanuprakashnani CreditAttribution: bhanuprakashnani at Google Summer of Code commentedComment #5
ankitjain28may CreditAttribution: ankitjain28may as a volunteer and at Google Summer of Code commentedI would like to fix this.
Comment #6
ankitjain28may CreditAttribution: ankitjain28may as a volunteer and at Google Summer of Code commentedWhat should be the correct help text so i can write a patch ?
Comment #7
bhanuprakashnani CreditAttribution: bhanuprakashnani at Google Summer of Code commentedPlease give the correct text to be updated over there. Waiting for the reply. Patch will be uploaded as soon as I get the correct text. Thank you.
Comment #8
PasqualleThere is no "correct" text. Check the codebase, and suggest a better description, and we can discuss if it is good or bad.
Comment #9
ankitjain28may CreditAttribution: ankitjain28may as a volunteer and at Google Summer of Code commentedThis is the most appropriate patch.
Comment #11
PasqualleThere is no permission named "content". You made the description even worse..
Comment #12
joachim CreditAttribution: joachim as a volunteer commented> 'Users with the Administer content permission will be able to override these options.
Is there even one and only one permission that can be used here?
If not, we have two options:
a. say something general and vague, like 'Users with sufficient access rights will be able to override these options.'
b. add a description to each checkbox instead of to the checkboxes as a whole.
Comment #13
cilefen CreditAttribution: cilefen as a volunteer commentedThis is just opinion, but: Why show any help text there? Drupal doesn't routinely point out things you could do if you only had the permission.
Comment #14
ankitjain28may CreditAttribution: ankitjain28may as a volunteer and at Google Summer of Code commentedAs stated in comment #13, I have updated the text.
Comment #15
joachim CreditAttribution: joachim as a volunteer commentedThe patch looks a little broken...
Also, the EM tag here was because it was the name of a permission. We don't need to emphasise something that's just words.
Comment #16
mohit1604 CreditAttribution: mohit1604 at Google Summer of Code commentedDid changes as per comment#15.
Comment #17
ankitjain28may CreditAttribution: ankitjain28may as a volunteer and at Google Summer of Code commentedPatch #16 looks good to me and as expected in #15
Comment #18
ankitjain28may CreditAttribution: ankitjain28may as a volunteer and at Google Summer of Code commentedInterdiff of patch #14 and #16
Comment #19
bhanuprakashnani CreditAttribution: bhanuprakashnani at Google Summer of Code commentedPlease review the patch. I have already done this but didn't upload it.
Comment #21
bhanuprakashnani CreditAttribution: bhanuprakashnani at Google Summer of Code commentedsry for the wrong patch. got some unexpected errors. will correct them.
Comment #22
ankitjain28may CreditAttribution: ankitjain28may as a volunteer and at Google Summer of Code commentedThe patch in #14 was bit broken fix that in this patch.
Comment #23
PasqualleWhat you 2 guys doing here is really disturbing. I did not find anything related to your behavior in the d.o documentation, but let me give you my friendly advice:
- work in cooperation, this is not a competition
- understand the problem
- do not assign the ticket to yourself, core committer knows who worked on the patch. Assignment is only necessary if the problem require long time (like weeks) to solve and you are really working on it
- do not create interdiff patch for a small patch like this
- do not create a new patch (without any explanation) when there is one already for review
- do not blindly upload something, check that the patch is working
- too much comments without real value can be considered as spam
Comment #24
PasqualleComment #25
joachim CreditAttribution: joachim as a volunteer commented> - understand the problem
This, and also discuss the problem!
I proposed *two* possible solutions. The merits of each one should be discussed before we post patches!
Comment #26
ankitjain28may CreditAttribution: ankitjain28may as a volunteer and at Google Summer of Code commented@Pasqualle I am really sorry for my behavior, i will keep this in mind.
@joachim as @cilefen stated in #13 is much appropriate in my point of view, There isn't any need to put the help text there if it isn't pointing to anything.
Comment #27
bhanuprakashnani CreditAttribution: bhanuprakashnani at Google Summer of Code commented@Pasqualle
Pardon me for my actions. Would never do this again.
Comment #28
PasqualleHey, no problem. Don't let anyone stop you contributing to Drupal and open source..
Comment #29
joachim CreditAttribution: joachim as a volunteer commentedOK let's set this back to 'needs work'.
We need to discuss the merits of the two options I proposed in #12.
Is it feasible to list permissions for each of the checkboxes? Can anyone go through the node form and check what they are?
Comment #30
ifrikThanks Joachim,
I think we are safer in going with the first of the two options proposed in #12.
The main point about this description is to alert users that even though some default options are chosen, some users might nonetheless have the option to change this to something different. So the sitebuilder knows that they can set defaults that are not set in stone, and they also won't be surprised if user 1 or other users are able to change the settings even when they thought they were set here.
With further workflow option or so, other modules might add more checkboxes here, so a general description to alert the site builder would be future-proof.
Comment #31
Dinesh18 CreditAttribution: Dinesh18 as a volunteer commentedI agree with @ifrik, First option mentioned in #12 is much better approach and cleaner.
Comment #35
Rithesh BK CreditAttribution: Rithesh BK as a volunteer and at Valuebound for Valuebound commentedComment #36
Rithesh BK CreditAttribution: Rithesh BK as a volunteer and at Valuebound for Valuebound commented@Pasqualle i have tested the 2946177-16-D8.patch from #16 for 8.9.x-dev version. it is working fine. Please find the attached screenshot ...
Comment #37
PasqualleI agree, #16 looks good.
Comment #38
alexpottCommitted and pushed a56410bf7b to 9.0.x and 5a2738738e to 8.9.x. Thanks!