The new permalink variable is completely missing from Garland's comments template. This is a pretty big usability issue, IMO. And I see no obvious reason why it wasn't included. #819214: The "#" marking comment permalinks is just weird and untranslatable

Comments

johnalbin’s picture

Status: Active » Needs review
StatusFileSize
new2.45 KB

And here's a patch that puts the permalink directly next to the comment title.

johnalbin’s picture

StatusFileSize
new11.88 KB
new11.46 KB

Here's before and after shots.

Before:

After:

Status: Needs review » Needs work

The last submitted patch, 975254-garland-permalink.patch, failed testing.

johnalbin’s picture

Status: Needs work » Needs review

#1: 975254-garland-permalink.patch queued for re-testing.

markabur’s picture

#1: 975254-garland-permalink.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 975254-garland-permalink.patch, failed testing.

markabur’s picture

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

Reroll attached. Patch looks pretty good. This rule:

.comment .permalink {
  line-height: 200%;
}

causes the "Permalink" text to line up with the H3's baseline in Safari/Mac as in the screenshot above, but makes it a couple pixels low in FF/Mac. Removing the rule makes it line up properly in FF but makes it too high in Safari. I messed with a bit and couldn't find anything that worked better. I haven't looked at it in IE yet.

catch’s picture

Version: 7.x-dev » 8.x-dev
Priority: Major » Normal
Issue tags: +Needs backport to D7

Patch looks fine but this is not a major bug. Not sure whether it is sufficiently bad to break markup freeze but tagging for backport so it doesn't get lost at leat.

kscheirer’s picture

Issue tags: -Needs backport to D7

#7: 975254-garland-permalink-2.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs backport to D7

The last submitted patch, 975254-garland-permalink-2.patch, failed testing.

amateescu’s picture

Version: 8.x-dev » 7.x-dev
Issue tags: -Needs backport to D7

The Garland theme has been removed from D8: #911054: Remove Garland from Core

mahaprasad’s picture

Applied patch #7 & its failed.

for more info please find below details:

quickstart@qs10:~/websites/drupal7.dev$ patch -p1 < ../Patch/975254-garland-permalink-2.patch
patching file garland/comment.tpl.php
Hunk #1 FAILED at 5.
1 out of 1 hunk FAILED -- saving rejects to file garland/comment.tpl.php.rej
patching file garland/style-rtl.css
Hunk #1 FAILED at 233.
1 out of 1 hunk FAILED -- saving rejects to file garland/style-rtl.css.rej
patching file garland/style.css
Hunk #1 FAILED at 857.
1 out of 1 hunk FAILED -- saving rejects to file garland/style.css.rej
patching file garland/template.php
Hunk #1 FAILED at 131.
1 out of 1 hunk FAILED -- saving rejects to file garland/template.php.rej

I will re-rolled the patch #7 & upload the same.

Thank You!

mahaprasad’s picture

StatusFileSize
new1.85 KB

I have re-rolled #7 patch. Please review.

mahaprasad’s picture

Status: Needs work » Needs review
mahaprasad’s picture

StatusFileSize
new26.39 KB
new32.07 KB

Please find the tested screen shots.

Before Patch:
Before

After Patch:
After

mattih’s picture

Test passed.

Status: Needs review » 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.