Problem/Motivation

Visiting one's own contact form produces a 403 Access denied error, confusing users into believing the site is broken.

Contact form settings checkbox description text contains a link to a broken URL:
Contact form settings broken URL

Emails sent via a user's contact form contains a link to a non-working URL
Contact email broken URL

These links are unusable for ordinary users since there is a policy against non-admin users having access to their own contact form. Also there is not much point in giving such access anyway - users don't need to contact themselves. (Comment #57)
403 Error screen

Steps to reproduce

  1. Enable the "personal contact form" under "My Account" > "Edit".
  2. Clicked on the link that led to user/4/contact
  3. It says "Access denied You are not authorized to access this page." however there are no permissions for the personal contact form, but the account administrator can see this form at user/4/contact just like normal.

Proposed resolution

The primary goal is a Usability improvement: to eliminate the source of site-is-broken confusion for registered site users.

Option A:

Remove the links that trigger permission denied. See comments #9, #30, #35, #57, and patch in #9 and #57 and #63, Workarounds in #32 (form-alter ) & #43 (string override)). This solution in #63 was committed to Drupal 8, however this solution requires two core string changes, which David Rothstein (D7 maintainer) does not want to do for D7 stable unless absolutely necessary (Comment #124). Gisle in comment #125 argues that this is a major issue making a string change justifiable: "When clicking a hyperlink leads to a "Permission denied" page, it gives the end-user the idea that the site is broken, which is not the message I (as the designer of the site) wants to send."

Option B:

Ensure non-admin users can always see their own “Contact” tab and that the link to the page works, but prevent user from contacting themselves (Comment #11). Eg, show error message on form submit if from/to are the same stating: "For security reasons, you cannot use your own contact form.” (Comment #12) addd as #markup to the form (Comment #13) (Patch #20) and/or disable the "Send e-mail" button and output a short message explaining why it is disabled (Comment #29).

Option B tries to solve the larger usability issues: people want to see their functioning contact form, so they can link people to it; people may want to test the site to prove that email sending is working, and emailing yourself is a perfectly valid use case.

Caveats to Option B:
"For security reasons” — If you register and then change your e-mail address in your account, you could then use your own contact form to send spam to unwilling addresses. That is why it is not allowed (Comment #23). However, this is easy to circumvent by opening two accounts and then spam one from the other -- an unavoidable security problem, not one that exists solely from allowing people to email themselves (Comment #24 & 27). Forcing users to validate changed email addresses, would seem to be the only way to block this security hole. However, it should be noted that issue #601250: Allow anonymous users to use personal contact forms allows anonymous users to use contact forms, so it becomes an UX oddity that members can't access their own form logged in, but can anonymously.

Remaining tasks

Commit solution to D8
Backport a solution to D7.

User interface changes

Non-functional links will be removed from email messages and user profile form contact settings.

API changes

String changes will affect Drupal translations.

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Status: Active » Closed (works as designed)

You cannot access your own contact form.

OneTwoTait’s picture

Title: Access denied to personal contact form » Let users see and use their personal contact form!!
Category: bug » feature

Thanks for the info.

A lot of users are going to click that link and get the error. I'll put removing that link on the list of things for a designer/themer to edit.

Plus, I'll also have to add a emphasized note that clearly explains that only other users will be able to see your contact form.

Even after those changes I'm sure to get emails about "my contact form doesn't show up".

Why not just let them see and use their own form?

Dave Reid’s picture

Try logging in as one of those users. You shouldn't be able to see the link. On their own user account, users do not see a 'Contact' tab, and should not see any menu item for their own contact form.

If you are a user that has the permission 'administer site-wide contact form,' or you are user #1, you have access to see your own contact form, or anyone else's contact forms.

A user cannot access their contact form because I could change my account's e-mail address to anything I want, and then use my contact form to spam whatever e-mail address I want.

OneTwoTait’s picture

I got to that link while logged in as a normal authenticated user (not user #1).

It's on the description text on the account edit page. It goes like this:
[checkbox] "Personal contact form

Allow other users to contact you by e-mail via your personal contact form. Note that while your e-mail address is not made public to other members of the community, privileged users such as site administrators are able to contact you even if you choose not to enable this feature."

The "your personal contact form" part is a link that leads to the page with "Access denied" on it.

Dave Reid’s picture

Title: Let users see and use their personal contact form!! » Link to contact form in user account leads to 403.
Version: 6.6 » 7.x-dev
Category: feature » bug
Status: Closed (works as designed) » Active

Aha! Now I know what link you're talking about! I would def. classify that as a bug. Working on a patch for 7.x first, then will backport.

Dave Reid’s picture

Status: Active » Needs review
FileSize
1.95 KB

First attempt, I think there may be a better way to do this, but it works right now.

Dave Reid’s picture

Assigned: Unassigned » Dave Reid
mikey_p’s picture

I can't see a better way to do that, and wonder if we need a link there at all? Seems like its been there since contact.module was first committed.

Dave Reid’s picture

Yeah I might think it's just better to remove the link. If the user has access to their own contact form, they can see the dang "Contact" tab on their user page. There's no need to have that link - it's just confusing.

OneTwoTait’s picture

I agree, removing the link is the best way to go.

Damien Tournoud’s picture

There is a big usability issue here, very similar to #91663: Permission of text format is not checked when editing an entity and instead reset to something a user can use..

While we are at it, why not making sure that the user can always see its own "contact" tab, but simply disable the form in there? (adding a '#access' to the submit link would be more than enough... and that's nearly a one line fix).

Dave Reid’s picture

Damien, should we put a drupal_set_message('You can not use your own contact form.')?

Damien Tournoud’s picture

@Dave: that's a question to ask to the usability team. I personally think it would be nice, but I would rather embed it in the form (a '#type' => 'markup' element), because form elements can be form_altered, while drupal_set_message() can't.

Damien Tournoud’s picture

Status: Needs review » Needs work

Back to CNW.

Dave Reid’s picture

Dave Reid’s picture

Issue tags: +Needs usability review
rbgrn’s picture

Dave,

I have noticed this problem in D6.9 still. Basically - that text needs to not link to a spot that will be permission denied for all but the administrators.

Any chance we can get this patch into D6.10?

One more note - The default notification email template for new accounts in D6 also links users to their personal contact form which gives them an access denied. Perhaps that should be changed as well.

Thanks!

Bojhan’s picture

Issue tags: -Needs usability review

Is there another active issue on this?

pwieck’s picture

Is there a 6.x fix or patch yet. Just started allowing users and already getting mass emails about this problem.

Dave Reid’s picture

Status: Needs work » Needs review
Issue tags: +Needs backport to D6, +Needs usability review, +Needs backport to D5
FileSize
5.83 KB

Revised patch:
- Removes the link to the user's own contact form in their profile.
- Changes the access function for all personal contact forms to not check if the current and contacted user are the same
- If the current and contacted user are the same users, it will display a warning-style message at the top of the personal contact form that says "For security reasons, you cannot use your own contact form."

Can post screenshots shortly.

rhouse’s picture

I am puzzled about this solution. Re your three bullet points:

1) contacting your self might be pointless, but it is not inherently irrational. In fact, in order to test a site's email capabilities, it might be a sensible thing to do. Fundamental programming principle: never ban anything just because it seems pointless, only ban the irrational. (Example: I was getting infinite loops in the sort function from an early Microsoft C compiler every Sunday. Sunday there were no new items and thus nothing to sort. The library assumed that no one would ever sort nothing, and used an algorithm that infinite-looped. So special tests had to be inserted into the logic to skip the sort of no items. Smart way: no (or 1) items are always sorted, do nothing.) "X is pointless" is usually a failure of one's own imagination, not a valid reason to ban something.

2) This is correct.

3) If you say so, but I cannot imagine any earthly security reason for preventing someone emailing themself. I would very much appreciate being filled in on this, as obviously I am missing something of significance.

rhouse’s picture

A few more thoughts. I experimented with the code in D6, and discovered that if I also removed the test for user 0, the algorithm fell through and gave a "You need to provide a valid e-mail address to contact other users. Please update your user information and try again" message. Why not add code in this case to actually put a 'return address' box on the form and then allow it? That way, the site admin can choose whether to permit anonymous emailings or not. It might also be good to introduce another permission "access other site members contact form" as opposed to "access site-wide contact form" to control this, so the admin can choose with rationale something like "I am prepared to tolerate anonymous emails, but I don't want my readers pestered."

I would do this myself, but I don't know how to make the requisite return address box appear. And there is one problem, the hourly email limit. With anonymous users this would have to be run off something like the IP address of the user.

Dave Reid’s picture

@RTH
@21: If you register for a Drupal site and then change your e-mail address in your user account form, you could then use your own contact form to send spam to unwilling addresses. That is why it is not allowed. If ever the time comes that Drupal does force users to validate changed e-mail addresses, I'm sure we could open this restriction up.
@22: Personal contact forms can only be used by registered users. You attempted to use it as an anonymous user, which it was not meant to do. Please see #58224: Allow anonymous users access to a members personal contact form for that issue. Let's keep things separate.

rhouse’s picture

Thanks Dave.

Re @21: All someone has to do is open two accounts under different aliases, and then spam one from the other. It seems this is an unavoidable security problem, not one that exists solely from allowing people to email themselves. The other measure you mention, forcing users to validate changed email addresses, would seem to be the only way to block the security hole.

Re @22: Thanks for that, I wasn't aware of the other issue. In that case, while the other issue is being discussed, I suggest doing what I have done locally: removed the condition preventing user 0 seeing an email link (this causes the "You need to provide a valid e-mail address..." message), but then adding a new message in front of that one, testing for user 0 and saying something like "You must log in to send email to other users." By doing this, the fact that email is possible if you log in becomes visible and gives people a reason for registering on your site.

Bojhan’s picture

changing things

Dave Reid’s picture

@RTH: If a user opens a second account, they have to verify the e-mail used for their new account as well, so that doesn't work. I'm not going to do anything about checking user uid 0 because that changes everything about this issue and makes things more complex. Anything about that should be discussed in #58224: Allow anonymous users access to a members personal contact form. If you check out the patches in that issue you will see how we are solving the problem you are describing. Please keep this issue to *only* about improving the error message as is. Let's not derail an easy fix issue.

rhouse’s picture

Thanks Dave. I don't understand your remark about two user logins. As I understand it, the security hole is: User opens account, changes email address, then emails "himself" to send spam. Is that correct? If so, why cannot a user open account A and account B, change the email address for account B, and use account A to spam the new unverified target of B? I am missing why this security hole exists only in the case where a user may email himself.

Status: Needs review » Needs work

The last submitted patch failed testing.

gpk’s picture

Maybe this fix would get into core quicker if the code style improvements went into a separate issue? It's hard to spot the actual code changes.. :)

I'm not sure the spam problem is all that severe, especially with the flood constraint. But the harder it is to spam then the less it will happen (people will look for easier ways). Being able to view your own contact form would be a "nice to have" though, especially for less confident users who might find it reassuring to see what they are letting themselves in for ... perhaps disable the "Send e-mail" button and output a short message explaining why it is disabled.

Weka’s picture

I agree, the main problem is the visibility of the "your personal contact form" link in the description text under the "Personal contact form" check box for users that do not have the "administer site-wide contact form" permission.

Removing the link would solve this issue. Moving everything else into a separate issue is a good idea.

Please remove the link from 6.x.

Weka’s picture

Is there some alternate way around this issue until this gets fixed?
How can I get rid of the "your personal contact form" link in 6.14?

hefox’s picture

@Dinornis and anyone else looking for a fix till then: form_alter

function modulename_form_user_profile_form_alter(&$form, $form_state) {
    $form['contact']['contact']['#description'] = t('Allow other users to contact you via a personal contact form which keeps your e-mail address hidden. Note that some privileged users such as site administrators are still able to contact you even if you choose to disable this feature.');
}

where modulename is a name of a module. (I assume there's a tutorial somewhere on how to make a simply module).

Sorry for cluttering the issue; if I used the contact form to contact him someone would probably respond anyway.

sun’s picture

Issue tags: +API clean-up
gpk’s picture

Just to note that #601250: Allow anonymous users to use personal contact forms has been committed, superseding #58224: Allow anonymous users access to a members personal contact form.

If you do give anon users permission to access/use members' contact forms then it becomes even more of an oddity that members can't in general access their own and are simply presented with an access denied message.

bardolph’s picture

It seems the most obvious solution is to remove the link in the email. Why send out a link that the user can't visit? That's a bad user experience.

Please forgive the noobiness of this question, but how do I edit the email template that the contact form sends out? I've been clicking through the admin menus forever, and I can't find it anywhere.

steveray’s picture

Hi Dave,
I accidentally duplicate posted in #661060: Dead link generation with contact.module?, but find here that the question, while thoroughly discussed, has not been resolved for D6.

Is there a way to simply remove the 403 link from the form in D6 without writing a new module or hacking/patching core?

All other usability issues aside (imho), just eliminating the source of site-is-broken confusion for the end-user is the best starting point (and possibly the best ending point, for D6 at least).

Steve

hefox’s picture

Other than by css? Probably not to my knowledge

It's really easy to do it n a module, I'd suggest just to do that :>

function blocked_form_user_profile_form_alter(&$form, $form_state) { 
    $form['contact']['contact']['#description'] = t('Allow other users to contact you via a personal contact form which keeps your e-mail address hidden. Note that some privileged users such as site administrators are still able to contact you even if you choose to disable this feature.');    
}

(from http://github.com/hefox/blocked)

ñull’s picture

My suggestion is to change the functionality of the contact form when it is your own: let it include a destination ajax field that allows you to choose one of the users that have their contact form enabled (except yourself). That way the Contact remains functional (allowing to mail others that permit it) and at the same time it avoids mailing yourself. That way your own contact form should always be active to enable contacting others.

fgiacanelli’s picture

I have the same problem with a new community site on Drupal 6.15. After a copule of day a user told me he was puzzled to have got "access denied" on his own personal contac form from his "my account" page.

jtrant’s picture

Version: 7.x-dev » 6.9

this forbidden link is also in every email message sent with the personal contact form:

the default text says:

[addressedUser],

[sendingUser] (http://sitename.com/user/[sendingUser]) has sent you a message via your contact form (http://sitename.com/user/[addressedUserID]/contact) at sitename.

that emailed link is forbidden to the addressed user -- as it's their own contact form -- and should be removed from the message.

thanks.

Dave Reid’s picture

Title: Link to contact form in user account leads to 403. » Link to contact form in user account and e-mails leads to 403.
Version: 6.9 » 7.x-dev

Please don't change the version unless you know what you're doing. Needs to be fixed in D7 first.

jtrant’s picture

sorry -- version change wasn't intentional.

/jt

jwilson3’s picture

for those that need this now, rather than later, and dont want to hack core, the d6 quick-fix for the faulty email text is to use string overrides. Here is a one-liner that does just that... put it at the bottom of your settings.php file.

$conf['locale_custom_strings_en']['!name (!name-url) has sent you a message via your contact form (!form-url) at !site.'] = '!name (!name-url) has sent you a message via your contact form at !site.';
gpk’s picture

Callatya’s picture

The settings.php line throws errors in D6 for me.

Is there another option for just removing the link from the auto-generated email, and if not can someone please walk me through hacking the core?

jwilson3’s picture

@Callatya: use only the line of code between the <?php and ?> (ie dont include the <?php or ?> in your settings.php file), its used here only for syntax highlighting purposes.

mstef’s picture

Why was this changed 7.x from 6.x? It still exists in 6.x.

hefox’s picture

It will be fixed in 7 then back ported to 6/5, but 7 gets the ticket, which is generally the case.

(Anyhow, was looking for an excuse to say: there's an issue that is similar, if you edit your email and try to set it to an other user's email, it gives you that "Did you forget your password?" message linking to 403-ed user/pass >.O. Mentioning it here so don't forget, still looking to see if there's an existing issue queue post.).

vj_pdx’s picture

subscribe

vj_pdx’s picture

FileSize
58.16 KB

Attaching a screenshot, Drupal 6.19...

jwilson3’s picture

I'm assuming the image attached in #51 documents the error described in #49 somehow?? because the the url doesnt say user/X/contact, which should be a different issue than this one.

The issue in this thread is specifically that the link to your own contact form page in the email sent to you when someone sends you a message via the contact form gives you an access denied error if you're logged in to your account. Lets stay on topic here.

Would love to see a patch for d7 (has it been addressed yet? does anyone know?) so we can then backport to d6 and squash this #wtf usability issue in core.

MakeOnlineShop’s picture

And can you tell me how to completely hide this that users see when editing their account ? It is useless for an Ubercart shop:

Contact settings
Personal contact form
Allow other users to contact you by e-mail via your personal contact form. Note that while your e-mail address is not made public to other members of the community, privileged users such as site administrators are able to contact you even if you choose not to enable this feature.

Thank your for your help.

jwilson3’s picture

@make-online-shop:

Your suppor requests would be better served in the forums or on the support mailing list, this issue exists for fixing bugs in the code. See the Community section for more info on various ways to get help.

loopduplicate’s picture

I'm no Drupal master but I did manage to make a module for Drupal 6 (tested on 6.20) that implements the solution from comment #32. For anyone like me who is new at making a custom module, I uploaded it to my server and it's located here: http://rowdyrabbit.com/customdrupalmodules/contactzzz.tar.gz

This only solves the problem on the edit user page but doesn't solve the email problem. The email problem was successfully fixed by adding the line found in comment #43 to my settings.php.

apaderno’s picture

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

I guess that this issue needs now to be fixed in Drupal 8, first.

gisle’s picture

Status: Needs work » Needs review
FileSize
39.94 KB
13.71 KB
13.38 KB
2.05 KB

I am puzzled by this not yet being resolved. I think the required change is trivial and has already been mentioned several time in this thread. The problem is that the contact module automatically inserts a link to the users own contact page in the users contact settings form and in email messages sent the user when others uses the contact form. The link is unusable for ordinary users since there is a policy against non-admin users having access to their own contact form. Also there is not much point in giving such access anyway - users don't need to contact themselves.

I've created at patch for D7 based upon the 7.x branch that I think solves the issue (it is basically the same as already suggested). The patch is attached to this message and I hope someone is able to review it. I think it applies cleanly to the current 7.15 core. However, this is the first patch I submit, so please be gentle if I've not followed the correct procedure for submitting it, or made other mistakes.

To document the issue, three screenshots are attached (I've noticed the issue is tagged with "Needs screenshot".

  1. The first shows the confusing link in the contact settings form.
  2. The second shows the confusing link in the body of email sent using the contact form.
  3. The third shows the result of clicking on the link to one's own contact form (Access denied)

Issue tags: +Needs backport to D7

The last submitted patch, contact-remove-confusing-urls-345541-57.git_.patch, failed testing.

gisle’s picture

Status: Needs review » Needs work

OK, test failed - fair enough. Can some gentle person tells me how I goofed up with this patch (it applied cleanly when I tested it on my site).

loopduplicate’s picture

"Ensure the patch applies to the tip of the chosen code-base"
http://qa.drupal.org/pifr/test/359223#pifr-steps

Perhaps you didn't make the patch relative to the contact.module directory? Wish I could help more.

jwilson3’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs work » Needs review

The problem is that issue's "Version" field (currently 8.x-dev) needs to coincide with the patch... you can see this in the test log, that it used version 8.x to apply the patch, which obviously wouldn't apply cleanly.

                    [repository] => Array
                        (
                            [type] => git
                            [url] => git://git.drupal.org/project/drupal.git
                        )

                    [vcs_identifier] => 8.x

The standard coding practice dictates that the best option is to create a patch for Drupal 8, and then we could create a backport for drupal 7 (potentially using your patch in #57 as a guide or point of reference. I'll temporarily set this back to "7.x-dev" and retest (just to see if the test passes or not, so as to keep gisle motivated to keep working on it for D8, if they so choose ;)

After the test result comes up, this should be set back to "8.x-dev", and "needs work".

jwilson3’s picture

Version: 7.x-dev » 8.x-dev
Status: Needs review » Needs work

Hrm. apparently its not as easy as switching the version number and resetting to needs review, as this did nothing to trigger a re-test, (or maybe I just didnt wait long enough?)

anyway, switching it back to d8, needs work. Please provide a patch against d8.

apaderno’s picture

Status: Needs work » Needs review
FileSize
2.01 KB

This is the patch corrected for Drupal 8.

apaderno’s picture

Issue tags: -Needs backport to D5

I am cleaning the tags, as the code will not back-ported to Drupal 5.

gisle’s picture

This trivial issue has remained open for 3 years and 47 weeks!

IMHO, good patches exists for at least D7 and D8. But there has been at least two minor releases of D7 core since I became aware of the issue, and the existing patch is yet not merged into the dev branch.

I think that switching it to version 8.x-dev in comment #56 did not help, as very few people run D8, which means that there are few reviewers around, making it less likely that the patch gets to RTBC status.

I've been told that it need to be fixed in D8 first, then backported to D7, but that does not seem to happen.

I do, of course, have the patch. So I patch every minor release before updating my sites. But having a patch is not the same as having it in core. Patching means that I can't use automatic updates, and that I can't provide translations at http://localize.drupal.org.

This issue seems so trivial. Can somebody with more insight in the process than me do whatever is required to get this fixed?

hefox’s picture

Patches need to go into the highest applicable branch, and so issues should be associated with that branch (8.x). What most people do, I think, is set up a test 8.x install (and reinstall every so often as head to head isn't supported) to test patches.

Anyhow, patch applies and removes the link from the email and edit form.

In my opinion, it makes sense to do this instead of providing the form -- simpler and cleaner. I see the advantage of having the form, but not worth the extra logic.

gisle’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs screenshots, -API clean-up

Ok, I've downloaded and installed 8.x in order to test the patch in #63. (About time to get my feet wet with 8.x anyway.)

Patch in #63 applied cleanly to the now current version (SHA-1: 026508fb21fb29c0aeb734f1a3f1774bf38cd6db) of the 8.x branch.

It solves the bug by removing the link from the email and edit form. I agree with hefox #66 that this provides a simple and clean solution to the issue.

I am changing status to RTBC and cross my fingers, hoping this will make it into the next release.

I also removed the Needs screenshot tag (see #57 for screenshots).
I also removed the API cleanup tag. I see no need to clean up the API with this solution.

apaderno’s picture

#63: remove-link-345541-63.patch queued for re-testing.

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Nearly asked for tests for this, but it's sufficiently trivial I don't think it needs explicit test coverage. Committed/pushed to 8.x., moving to 7.x for backport.

dcam’s picture

Assigned: Dave Reid » Unassigned
Status: Patch (to be ported) » Needs review
FileSize
2.01 KB

Backported #63 to D7.

Status: Needs review » Needs work

The last submitted patch, 345541-70-remove-contact-link.patch, failed testing.

dcam’s picture

Status: Needs work » Needs review
FileSize
2.01 KB

Fixed the error in #70.

alberto56’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

This is a fine, simple patch which does what expected. In my opinion we don't need a test for this.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

alberto56’s picture

Status: Needs work » Needs review
alberto56’s picture

Status: Needs review » Reviewed & tested by the community

Setting back to RTBC, there was an error with the testbot itself. The patch is now passing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

David_Rothstein’s picture

Status: Needs work » Reviewed & tested by the community
alberto56’s picture

alberto56’s picture

Not sure why the test is failing, I set it to re-run.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

dcam’s picture

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Re-setting RTBC status after random test failure.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

alberto56’s picture

Status: Needs work » Needs review
alberto56’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

alberto56’s picture

Status: Needs work » Needs review
alberto56’s picture

Status: Needs review » Reviewed & tested by the community

OK, back to RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Wow, we're nearly up to 20 comments on this issue caused by the random test failures.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

Status: Needs work » Needs review
dcam’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 72: 345541-72-remove-contact-link.patch, failed testing.

alberto56’s picture

Status: Needs work » Needs review

Maybe a testbot glitch

dcam’s picture

Status: Needs review » Reviewed & tested by the community
David_Rothstein’s picture

Status: Reviewed & tested by the community » Needs work

Unfortunately this patch changes end-user-facings (not admin-facing strings) which we don't want to do in a stable release unless it's a very serious problem (see https://www.drupal.org/node/1527558).

I think we should explore other solutions for this that might be backportable, such as showing some version of the contact form to the user after all, possibly blocked from submitting (e.g. as described in #11)? This might be better in a separate issue (and just close this one) since it would need to go into Drupal 8 also.

marco.b’s picture

As the link is in a html description I wonder why that is so difficult, but I'm just a sitebuilder and still have little insight ;-(

A non-code workaround that I use is to disable the "Contact user element" with Display Suite Forms. I recognize that this workaround prevents users from the option to disable their user-contact form. In almost any cases I don't need that option, so I configure site-wide with the drupal permissions if user-contact forms should be displayed or not, so it works fine.

gisle’s picture

Status: Needs work » Needs review
FileSize
2.01 KB

Just testing.

gisle’s picture

Status: Needs review » Reviewed & tested by the community

Automatic test passed.

mgifford’s picture

So @gisle this is the same patch as in #72?

gisle’s picture

So @gisle this is the same patch as in #72?

Yes it is.

Because of this engagement: #2667218: The output from Drupal.org automated tests should be useful for humans, I wanted to see how the revamped automatic test system responded this.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Needs work

So it still should be "needs work" for #118 then, right?

gisle’s picture

Priority: Normal » Major

So it still should be "needs work" for #118 then, right?

IMHO, no.

While I understand that this break translations (which should not be done unless the problem is at least major), this bug is a major problem: The affordances of a hyperlink is that it is clickable. When clicking a hyperlink leads to a "Permission denied" page, it gives the end-user the idea that the site is broken, which is not the message I (as the designer of the site) wants to send.

Broken translations are fixed globally as soon as they're pushed to localize.drupal.org - and can also be fixed locally by the site maintainer - i.e. a broken translation is not a big deal.

I can live with this (by applying the path in #72 before deploying a new version of the core on a production site). But leaving this major UX bug in core unfixed for seven years is IMHO really bad for the reputation of Drupal.

jwilson3’s picture

Issue summary: View changes

Felt the need to clean up the issue summary. There are essentially two options presented in the issue, which I've labeled Option A and Option B based on the order they were presented in.

In summary: Option A was committed to Drupal 8. David argues that Option A breaks strings, which should be avoided in Drupal 7. Gisle argues that breaking the string in this case is OK because the behavior makes the Drupal site "seem" broken to end users. Hefox in #66 comments that he sees the value in Option B but that it's just not worth writing the extra logic to make it work. I think this is still debatable: invalidate all language files, requiring a trivial level of effort from a large group of translation team people, versus a small group of people working on a non-trivial problem (ie, writing additional form logic to allow access to the form, get usability input and sign-off, and writing tests).

jwilson3’s picture

After reading through all the comments on the issue in order to update the summary, my conclusion is that the better fix is to change the strings in Drupal 7, yes this causes more work for translation teams, but I think the bulk translation work would be less than the level of effort to enable one's own contact form. This is evidenced by the back-and-forth in the comments above, the circular discussions about usability/security etc etc... and had this been an easy change, then it would have clearly made it into Drupal 8 as the definitive solution over the trivial string change.

  • catch committed e482865 on 8.3.x
    Issue #345541 by Dave Reid, gisle, kiamlaluno: Link to contact form in...

  • catch committed e482865 on 8.3.x
    Issue #345541 by Dave Reid, gisle, kiamlaluno: Link to contact form in...

  • catch committed e482865 on 8.4.x
    Issue #345541 by Dave Reid, gisle, kiamlaluno: Link to contact form in...

  • catch committed e482865 on 8.4.x
    Issue #345541 by Dave Reid, gisle, kiamlaluno: Link to contact form in...
tekNorah’s picture

Is this issue being tracked somewhere else? This just happened when someone tried to contact me.

apaderno’s picture

@tekNorah No, it is just handled here. Still, there isn't any acceptable patch for Drupal 7; that is why it still happens on Drupal 7. Drupal 8 has been fixed already.

gisle’s picture

It is fixed for Drupal 7. Core maintainers just refuse to accept the working fix because it "breaks translation". But there is, AFAIK, no way to fix this without editing the string - so we're stuck with this (nine years and counting).

apaderno’s picture

I don't see alternatives, if not editing the string to remove the part containing the link made, using code. It seems quite silly to dynamically edit a literal string just to avoid introducing a new translatable string.

apaderno’s picture

Issue tags: -Needs backport to D6

Maybe I am giving a too loose interpretation, but basing on what written in https://www.drupal.org/node/1527558, should not the string be considered as giving wrong information? At least it induces users to believe they can access their contact form because they are used to Drupal not showing them links to pages they cannot access.

String change (user-facing) Any changes or additions to any user-facing string (string wrapped in t() or st()); for example comments or the log in form. Forces translators to translate new/changed strings, and multilingual sites see English strings until this is done. Major/Critical string problem only. (e.g. typo, string is saying completely wrong information)

I understand that changing a string used for the user interface just because I don't like it is a no, but in this case there is a bug to fix, and the bug is exactly what the string is showing to users.

I also understand it still possible to change the strings passed to t() from the settings.php file, but would users be expected of doing this for every buggy string used in Drupal core?

tekNorah’s picture

Just to be clear, this is affecting drupal.org. So, shouldn't that make it a priority? At the very least, who do we tag that is Admin on drupal.org so they can apply a patch?

apaderno’s picture

Issue tags: +affects drupal.org
tekNorah’s picture

Thank you kiamlaluno

dunx’s picture

11 years! Oh dear d.o

apaderno’s picture

Issue tags: -Needs backport to D7
jmizarela’s picture

As commenter @dunx mentioned (#140), we have had this 403 link for way more than a decade. I believe this is an indication that the link is not essential and should be removed as stated in Option A, what, I know, was attempted and not implemented in the end. So, what I mean by removing is, as the Drupal site moves on from Drupal7, this link should probably not be on it's Drupal9/10 version. I'm not sure how to track this issue during this migration, but wanted to bring visibility/discussion back to this issue.