Problem/Motivation

The localization string for the "Submitted by" node field (NodeSubmittedBy) is:

Submitted by <a href=":user_link">@user</a> on @date.

This leads to two problems.

First, a user without permissions to view user profiles may be presented with a link that leads to an access denied page.

Second, the user name is already rendered as a link when the current user has access to the user profile, which leads to invalid HTML output like the following:

Submitted by <a href="/users/someone"><a title="View user profile." href="/users/someone" about="/users/someone" typeof="schema:Person" property="schema:name" datatype="" class="username" lang="">Someone</a></a> on Wednesday, June 29, 2016 - 14:01.

Proposed resolution

Remove the link from the text passed to ::t().

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#16 ds_submitted-by_2853906_16.patch2.29 KBJelle_S
#14 ds-2853906-do-not-test.patch2.29 KBJelle_S
#13 ds_submitted-by_2853906_13.patch2.29 KBJelle_S
#12 ds_submitted-by_2853906_12.patch2.32 KBwesleydv
#9 ds_submitted-by_2853906_9.patch789 byteswesleydv
#6 ds-submitted-by-2853906-6.patch2.14 KBnedjo
#6 interdiff.txt1.38 KBnedjo
#6 ds-submitted-by-2853906-6.patch2.14 KBnedjo
#4 ds-submitted-by-2853906-4.patch2.02 KBnedjo
#2 ds-submitted-by-2853906-2.patch802 bytesnedjo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nedjo created an issue. See original summary.

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented
Status: Active » Needs review
FileSize
ds-submitted-by-2853906-2.patch802 bytes

Status: Needs review » Needs work

The last submitted patch, 2: ds-submitted-by-2853906-2.patch, failed testing.

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented
Status: Needs work » Needs review
FileSize
ds-submitted-by-2853906-4.patch2.02 KB

Tweaking the existing test. Ideally we'd also test with a non-admin user lacking access to user profiles.

Status: Needs review » Needs work

The last submitted patch, 4: ds-submitted-by-2853906-4.patch, failed testing.

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo commented
Status: Needs work » Needs review
FileSize
ds-submitted-by-2853906-6.patch2.14 KB
interdiff.txt1.38 KB
ds-submitted-by-2853906-6.patch2.14 KB

User profile link is rendered without a span.

wesleydv’s picture

wesleydv CreditAttribution: wesleydv at District09 commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.
Because showing, or not showing, the link is now handled by the user entity rendering I don't think it needs a separate test.

swentel’s picture

swentel CreditAttribution: swentel as a volunteer commented
Status: Reviewed & tested by the community » Needs work 
+++ b/src/Tests/EntitiesTest.php
@@ -70,7 +70,8 @@ class EntitiesTest extends FastTestBase {
+    // Because the user has 'access user profiles' permission, user name is rendered in a link.

comment shouldn't exceed 80 lines

wesleydv’s picture

wesleydv CreditAttribution: wesleydv at District09 commented
FileSize
ds_submitted-by_2853906_9.patch789 bytes

Patch #6 does not apply anymore, because the EntitiesTest does not exists anymore

wesleydv’s picture

wesleydv CreditAttribution: wesleydv at District09 commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 9: ds_submitted-by_2853906_9.patch, failed testing. View results

wesleydv’s picture

wesleydv CreditAttribution: wesleydv at District09 commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
ds_submitted-by_2853906_12.patch2.32 KB

Apparently the test moved to Drupal\Tests\ds\Functional\EntitiesTest updated the patch to change the test there.

Jelle_S’s picture

Jelle_S
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: Jelle_S at District09 commented
FileSize
ds_submitted-by_2853906_13.patch2.29 KB

Rerolled #12.

Jelle_S’s picture

Jelle_S
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: Jelle_S at District09 commented
FileSize
ds-2853906-do-not-test.patch2.29 KB

Previous reroll was against dev, this one is for 3.2.

Status: Needs review » Needs work

The last submitted patch, 14: ds-2853906-do-not-test.patch, failed testing. View results

Jelle_S’s picture

Jelle_S
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: Jelle_S at District09 commented
Status: Needs work » Needs review
FileSize
ds_submitted-by_2853906_16.patch2.29 KB

Retolled patch #13

  • swentel committed f736142 on 8.x-3.x 
    Issue #2853906 by nedjo, Jelle_S, wesleydv: Submitted by field...

  • swentel committed 77578ac on 8.x-4.x 
    Issue #2853906 by nedjo, Jelle_S, wesleydv: Submitted by field...
swentel’s picture

swentel CreditAttribution: swentel at eps & kaas for MuseScore commented
Status: Needs review » Fixed

committed to 8.x-4.x and 8.x-3.x and pushed, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.