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.
I'm not sure how to go about this, but would it be possible to have the untruncated log entry title as a title attribute on reports/dblog? That way, you don't have to click on entries with long titles in order to find out what they are.
Comment | File | Size | Author |
---|---|---|---|
#30 | interdiff.txt | 915 bytes | benjf |
#30 | 466416-30.patch | 1 KB | benjf |
#28 | 466416-28.patch | 1012 bytes | benjf |
#22 | 466416-22.patch | 915 bytes | mgifford |
#18 | 466416-3.patch | 0 bytes | mgifford |
Comments
Comment #1
RobLoachI find this very helpful when the log entries are clipped. Here's both a patch and a screenshot....
Comment #2
aspilicious CreditAttribution: aspilicious commentedI like this change, less clicks is always better
Code looks good.
Someone of the design team needs to give their opinion.
But in my opinion this can be labelled RTBC when bot is green.
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commentedwhy are we truncating the 'title' version? seems lie title version should get strip_tags as well since no browsers support tags in title popups
Comment #4
RobLoachIs there a limit to how long an HTML title attribute can be? Also, does this protect against " and ' HTML injection?
Here's a related issue: #718662: DBLog listings truncate messages in the middle of HTML tags
Comment #5
cosmicdreams CreditAttribution: cosmicdreams commentedTested this out in Chrome 5 beta. Worked great. I had a huge security warning when drupal 7 couldn't write to my .htaccess file. It was a real timesaver to not have to click on that link to see the actual warning.
This is one of those things that bugs admins a lot, but not frequently enough to file a formal complaint about. THANKS A TON!
Comment #6
webchickI'd like to get the accessibility team to chime in here. Too often we go title crazy and it makes pages totally inaccessible for blind users.
Comment #7
Everett Zufelt CreditAttribution: Everett Zufelt commentedI notice questions in comments #3 and #4 without any answers. Don't really think this patch is RTBC before they receive responses.
If my understanding is correct this patch will set the title attribute of the anchor pointing to a blog post to the full title of the post when the title in the anchor's link-text is truncated. If so then I don't see a accessibility problem, just that some people won't have access to this additional information, which is mostly redundant anyway.
Comment #8
RobLoachPushing to critical as it seems you can inject HTML within the log page. Try this using the PHP filter by Previewing a node, and then visit admin/reports/dblog:
Comment #9
RobLoachWow, I need more coffee. You're suppose to be able to have HTML in the link contents, just not the title attribute. This patch uses strip_tags to remove the tags from the title attribute.
moshe @ #3:
We don't want an HTML attribute that ends up being thousands of characters long. Like any code, we should not trust input, and protect against excessively long title attributes. This patch trims it to 256 characters, which seems like a reasonable amount (see screenshot). Without trimming, the hover tag just looks stupid.
Rob Loach @ #4:
This patch uses strip tags to protect against HTML injection, and remove the tags from showing up in the title attribute.
Everett Zufelt @ #7:
Well, that should just about do it!
Comment #10
Everett Zufelt CreditAttribution: Everett Zufelt commented+1 here for accessibility. Kind of annoying that we can't offer a longer link-text for users who cannot access the tooltip, but it would be a lot of work for very little benefit.
Comment #11
Dries CreditAttribution: Dries commentedLowering priority to 'normal'. Critical bugs are release showstoppers. This wouldn't hold back a release.
Comment #12
amc CreditAttribution: amc commented#9: 466416.patch queued for re-testing.
Comment #14
mgiffordtrying to keep up with core. there was a themable function added here.
I believe the required functionality is moved over but it needs to be checked.
Comment #16
mgiffordSimpletests need to be addressed. think the title is throwing off the verification.
Comment #17
mgiffordWe can put this in to the hopper for D8 at this point.
Comment #18
mgiffordUpdate of patch for new file structure. It still needs new simpletests.
Comment #20
mgiffordFrom AllySprint.
Comment #21
bowersox CreditAttribution: bowersox commentedI agree with Mike's un-tagging and comment. This issue is not an accessibility problem. The issue is a good enhancement idea that would help admins. If the tooltip is added using a
title=""
attribute, that would be fine. That is the feeling of us here at the Accessibility Sprint in Montreal.Comment #22
mgiffordre-roll.
Comment #24
mgifford#22: 466416-22.patch queued for re-testing.
Comment #26
Fabianx CreditAttribution: Fabianx commentedSounds like a task to me.
Comment #27
mgiffordpatch no longer applies.
Comment #28
benjf CreditAttribution: benjf commentedre-roll to move this to the Controller file.
Comment #30
benjf CreditAttribution: benjf commentedre-roll with a small fix, which should now pass the tests. Cottser++
Comment #31
Fabianx CreditAttribution: Fabianx commentedRTBC, looks great to me
Comment #32
tstoecklerI think it only makes sense to add a
title
attribute if the message was actually truncated, i.e. if the truncating actually changed the message. Or is that too much of an edge case that it's not worth adding complexity for it?Comment #33
star-szrComment #34
mgiffordI think it would be more consistent for the user if the title attribute was available regardless.
Alternatively we can address that in a follow-up issue. This has been an open & solvable issue for 5 years.
@tstoeckler we can open this up for longer if you think this needs more discussion.
Comment #35
tstoecklerNope, that totally works for me. :-)
Comment #36
alexpottCommitted a4109b3 and pushed to 8.0.x. Thanks!