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.
The Logging and Errors page has a setting to decide what kinds of error messages will be displayed on the screen. The setting is named "Display PHP messages", which is kind of misleading, and is also missing a description for its options. I suggest we:
- change "Display PHP messages" to "Error messages to display"
- add a description to the setting
Comment | File | Size | Author |
---|---|---|---|
#16 | fix_log_level_ui_00.patch | 1.15 KB | Xano |
#9 | error_msg_settings.patch | 1.08 KB | webkenny |
#4 | 569276_error_msgs.patch | 1.01 KB | reglogge |
#4 | 569276_error_msgs.png | 56.02 KB | reglogge |
#2 | new-error-setting.JPG | 17.46 KB | joshmiller |
Comments
Comment #1
lut4rp CreditAttribution: lut4rp commentedComment #2
joshmillerAttached is a screenshot.
I think the title might better read "Display Error Messages." I really like the new description.
The patch code is RTBC. Applies with a little fuzz, but nothing bad.
Comment #3
yoroy CreditAttribution: yoroy commentedPlease remove the first sentence of the description, it only repeats the label thus not offering any new information.
leaves:
__
Sites running in a production environment are recommended not to display any errors, but they may be helpful in a development or testing environment.
__
"they" seems to refer to the sites, not the errors?
If this lets you choose which ones to show, is it necessary to suggest when they'd be helpful at all?
Why not just:
__
Sites running in a production environment are recommended not to display any errors.
__
Not sure if that's the most concise way of putting it but that's all we need to tell I think.
Comment #4
reglogge CreditAttribution: reglogge commentedNew patch with suggestions from #3
Screenshot attached
On another note: The default value in PHP error reporting is set to ERROR_REPORTING_DISPLAY_ALL. I think it would be safer to set the default to ERROR_REPORTING_HIDE, so that inexperienced site-owners have to actually turn error reporting on if they want the world to see those pesky error messages.
Opinions?
Comment #5
reglogge CreditAttribution: reglogge commentedenabling bot
Comment #6
webkenny CreditAttribution: webkenny commentedMaybe I'm just being picky but I feel like the sentence, "Sites running in a production environment are recommended not to display any errors" is a little difficult in it's read.
What about something a little more friendly like "It is recommended that sites running on production environments not display any errors."
Just seems a little easier off the tongue, IMO. (Also cleaned up the grammar a touch)
Also, it's hard to know if the plural should be used or the singular. (e.g. "that a site running on a production environment"). But I'll leave that open for discussion.
Comment #7
yoroy CreditAttribution: yoroy commentedI agree that current sentence is cumbersome and I like the suggestion too, with one question (from a non-native speaker):
suggested in #6:
It is recommended that sites running on production environments not display any errors.
What I was missing from that sentence is *do* not display:
It is recommended that sites running on production environments do not display any errors.
I know the first is correct, too, but it reads a bit more abstract to me.
Comment #8
webkenny CreditAttribution: webkenny commentedAgree. The do does make it clearer. Both are correct but the 2nd makes it easier to read (and likely translate).
Comment #9
webkenny CreditAttribution: webkenny commentedGiving that a whirl. Patch attached.
Comment #10
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD.
Comment #12
Xano1) It is not misleading, because this setting actually only applies to PHP's messages (notices, errors, warnings and such)
2) Form elements do not necessarily need a description. Also, we decided not to tell site admins how to set up their website from within Drupal, especially when it's about development site versus live site options.
I suggest we revert the entire patch.
//Edit: Yes, reverting the patch requires a string change (and one string removal), but the current string just doesn't make clear what the setting does. One would have to check the source code to find it out.
Comment #13
Bojhan CreditAttribution: Bojhan commentedCore freez, non critical
Comment #14
XanoThis is a bug, and although it's not critical, bugs will still be fixed until Drupal 9 comes out.
Comment #15
Bojhan CreditAttribution: Bojhan commentedComment #16
XanoBAM!
Comment #17
XanoComment #18
XanoCan we give this issue a boost before RC1 comes out and we're in string freeze? Note that this is about a (minor) bug, because what the UI text says about the configuration option is a bit misleading.
Comment #19
Xano