Opened 8 years ago
Closed 6 years ago
#39165 closed task (blessed) (duplicate)
Add page to assist with debugging and support
Reported by: | jorbin | Owned by: | Clorith |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Administration | Keywords: | has-ui-feedback has-patch |
Focuses: | Cc: |
Description
Often, there is information that we want to know from people reporting issues. A simple page that includes info such as versions of software, extensions, dropins used, mu-plugins, etc could help with debugging and support.
Attachments (28)
Change History (125)
#4
@
7 years ago
Any way I could help? We talked about this at WCEU contributions day, so I know there is some work being done on this. I'd like to help with this, because I think that's an awesome idea.
#5
@
7 years ago
- Owner set to Clorith
- Status changed from new to assigned
What information is of importance to you as a supporter, we're already looking at inspiration from plugins like WooCommerce and Query Monitor, but that may not cover all scenarios.
Also, what information is of relevance to you as a user that you may need in general (like checking on a requirement in an easier manner)
The idea is to create a directly (for admins) accessible info.php
in core, much like the current options.php
.
Thoughts for consideration, should the page be hookable so that plugins can use it as well for certain debug data, to avoid having multiple locations for looking up information when looked at from a users perspective?
#7
@
7 years ago
I like the suggestions:
info.php
- "should the page be hookable so that plugins can use it as well for certain debug data, to avoid having multiple locations for looking up information"
The subpage could contain Report a bug button and submit system data.
This ticket was mentioned in Slack in #core-php by rabmalin. View the logs.
7 years ago
#10
in reply to:
↑ 9
@
7 years ago
@SergeyBiryukov
Ah, I didn't see this ticket, sorry. Yeah I'm very much in support of this idea. Would be very useful to include this in Core so there is a central place for debugging etc.
I'll see if I can get some kind of mockup of some kind.
#11
@
7 years ago
First pass at an info page, the fields aren't finalized yet, as it's still of interest to get some input on what data is of interest (bringing this up in the weekly support chat as well), and an easy to use copy field should also be added I think, but it may require some more considerations as you may not want to copy all the information that you'd normally show to a site owner when it's meant to just be pasted for support without a second thought.
Screenshot attached as well showing the current output from the patch before the copy field is introduced.
Also adds in the proposed filter for plugins and themes to add their own information. I opted to not include the core information in this filter, as we don't want anything to change the base information for a site.
#12
@
7 years ago
39165.diff changes the following:
- Add some information about the size of the setup (particularly for multisite): User Count, Site Count, Network Count
- Wrap some strings into
__()
- Fix to deprecated notices when calling
get_bloginfo( 'home' )
andget_bloginfo( 'siteurl' )
Let's keep in mind that the is_super_admin()
check should also be replaced with an actual capability before eventual merge (see #37616). But I didn't bother doing that yet, since this is only a draft at this point.
Furthermore regarding the information, I think the array should be using actual keys and have the field labels translatable. But again, not yet important for the draft.
#13
@
7 years ago
I agree that the array should have keys (which will actually be somewhat needed to introduce a private
, or similar, option to hide some fields from the copy-paste field.
I must admit that having the labels translatable hadn't crossed my mind, that one is a bit difficult, as in many situations you want debug information to be identical regardless of what language is used. I recall a discussion, I think it was the Japanese community, that mentioned that translating technical terms and jargon was a bad experience. Would love some input on that aspect of it but am totally open to making them translatable if there's a wish for it.
This ticket was mentioned in Slack in #forums by clorith. View the logs.
7 years ago
#15
@
7 years ago
transport can be fsockopen() as well, not just cURL, openssl version linked with PHP (extension openssl) is important then
and maybe add indication of FS_METHOD constant
#16
@
7 years ago
A language tweak:
'description' => __( 'The options shown below are relating to your server setup, and may require the help of your host if changes are required.' ),
Should be more like:
'description' => __( 'The options shown below relate to your server setup. If changes are required, you may need your web host's assistance.' ),
Breaking up the sentences for clarity :)
#17
@
7 years ago
So 39165.2.patch moves it over to an associated array as mentioned, and introduces the optional private
boolean field which will prevent text from showing in the textarea for copy-pasting (in the example, site URLs and database information are set to private).
Some discussions on translations occurred as well, what are the thoughts on Label (Translated label)
, this ensures the field is understandable by as many as possible, yet helps the site owner with clarity as they can get the label in their own language as well?
#18
follow-up:
↓ 29
@
7 years ago
I would like to see some more info. max_input_vars
for example (there are issue with the small default value 1000 and it pops up from time to time in the forums) and I would like to see if the PHP is 32 Bit or 64 Bit, because it could cause issues too (try absint
with a veeeery large id on a 32-bit PHP)
Would it be useful to collect plugins which already do this? There are plenty of those in the plugin directory ...
#19
@
7 years ago
Some people ask images related questions, it will be nice to see if "imagick" extension is present on their server, or WP_Image_Editor fallbacks to GD
#21
@
7 years ago
Here are few suggestions:
- Seperate the list of plugins into active- and inactive plugins. Usually when debugging we need to see list of active plugins, so I think this separation would make it easier for the user.
// List all available plugins $plugins = get_plugins(); foreach ( $plugins AS $plugin_path => $plugin ) { $key = ( is_plugin_active( $plugin_path ) ) ? 'Active plugins' : 'Inactive plugins'; $info[$key]['fields'][] = array( 'label' => $plugin['Name'], 'value' => sprintf( 'Version %s by %s', $plugin['Version'], $plugin['Author'] ) ); }
- Change "Other themes" to "Available themes", because I understand "Other" to be the list of all themes, excluding the active theme.
- I like how it's done in WooCommerce Status, where we get the number of active plugins too. So I suggest a
show_count
attribute:
'Available themes' => array( 'fields' => array(), 'show_count' => true ), 'Must Use Plugins' => array( 'fields' => array(), 'show_count' => true ), 'Active plugins' => array( 'fields' => array(), 'show_count' => true ), 'Inactive plugins' => array( 'fields' => array(), 'show_count' => true ),
and then e.g. when printing out values:
if( isset( $details['show_count'] ) && (bool) $details['show_count'] ) { printf( '<h2>%s (%d)</h2>', esc_html( $section ), count( $details['fields'] ) ); } else { printf( '<h2>%s</h2>', esc_html( $section ) ); }
So instead of these headings:
Other themes
Must Use Plugins
plugins
we have:
Available themes (32)
Must Use Plugins (1)
Active plugins (15)
Inactive plugins (88)
I'm working on this patch.
I like the private fields, that are not available in the copy/paste code, since users might post this on public forums.
This means we could also add e.g. the installation path as a private field.
#22
@
7 years ago
39165.4.patch implements proper keyed arrays, translatable strings (let's leave it up to the various languages to determine how to display technical terms and such).
It also includes the key count from @birgire, and the fields mentioned by others in the ticket as useful to include.
The is_super_admin()
check has also been replaced with a capability check, in our case update_core
, as a very reliable rule for someone who has the access expected when being privy to any technical information about the site setup.
#23
@
7 years ago
Thanks for the update @Clorith
just few additional ideas/remarks that come to mind:
- Table header (optional) ? See table-striped.JPG
- Striped table for easier read, i.e. alternating odd/even background? See table-striped.JPG
- "Not defined" vs "Undefined" (I like the latter more)
- If multisite is active then what about an extra multisite table?
- Table Of Contents - for easier navigation? (horizontal or vertical)
- "Go to the top of the document" links?
- How to close the copy&paste field again?
- Mention that the private info is excluded in the copy&paste field?
- Add WP_MEMORY_LIMIT ?
- Could we avoid constants like TEMPLATEPATH and WP_PLUGIN_DIR in favor of functions?
- The "Active theme" is included in "Other themes" (exclude it there?)
@
7 years ago
Example screenshot: table of content (by default the notice-box is using javascript for top position)
#24
@
7 years ago
Excellent feedback there.
- Table headers I'm not super fond of, since the data is very varied in a listing such as this.
- Multisite already does add a new table if enabled :)
- Closing the copy-field again was left out to avoid having multiple actionable items at once (much like WooCommerce does it as well)
39165.5.patch addresses the changes I am on board with:
- Striped tables are a must for distinguishing rows, that's going right in there.
- I agree that
Undefined
sounds better, so replacing those strings. - I also like your inclusion here of a ToC like quick-link bar along the top, I was a bit skeptical at first but they actually seem rather useful and your concept image really helped nail that down.
- Back to top links, if a ToC is added there will definitely need to be one of those to help with navigation.
- Moved the memory limit field, it was shown in the
WordPress
section still - The
TEMPLATEPATH
constant we can use the function for instead, as it's filterable it makes sense, butWP_PLUGIN_DIR
doesn't have a similarly associated function (unless I'm missing something) - Active theme should be filtered out from the general list of available themes, I agree, good catch.
- I've added a little section outlining that some information may not be included in the copy-field as it is considered private and should not be posted on public forums, but I'm not the best at using my words. It has the string
Some information may be filtered out from the list you are about to copy, this is information we consider private, and is not meant to be shared in a public forum
.
This ticket was mentioned in Slack in #core by clorith. View the logs.
7 years ago
#27
@
7 years ago
Such a page would be hugely beneficial.
Some additional information I would love to see:
- IPv6 support
- active mysqli driver
- active drop-ins
- maybe permissions for the main folders?
Also, I was thinking it might be better still if the page could generate an expiring link that will display this at the frontend. This would allow you to copy this link and provide it to the support rep you're talking to. If the link contains an expiring token, the security risk should be minimal.
#28
@
7 years ago
What would IPv6 support look like, the only thing I could think of here is if you are browsing the site over IPv6 your self?
The other three points are very valid, adding those now.
As for expiring links, that doesn't help over time, having the copy and paste field at the top is better here when users can put information right into their support topic or whatnot without the support giver being tied in to a time limit (if you post on the forums looking for help for example, there's no guarantee someone will click your link in the next X hours for it to remain valid).
#29
in reply to:
↑ 18
@
7 years ago
Replying to zodiac1978:
Would it be useful to collect plugins which already do this? There are plenty of those in the plugin directory ...
Maybe this list is useful to find some more approaches on this topic:
https://wordpress.org/plugins/server-info/
https://wordpress.org/plugins/rs-system-diagnostic/
https://wordpress.org/plugins/easy-debug-info/
https://wordpress.org/plugins/system-report-and-phpinfo/
https://wordpress.org/plugins/debug-info/
https://wordpress.org/plugins/wp-system-info/
#30
@
7 years ago
@Clorith thanks for the detailed feedback. Here are some minor adjustments:
- I get extra indentation for the first section (because the PHP tags are not like
<textarea><?php
and?></textarea>
). Maybe this was the intention for the first section?:` ### WordPress ### Version: 4.9-alpha-41375 Language: en_US
- The string
'version %1$s by %2$s'
is not translatable forwp-themes
, because of missing__()
. - We might want to wrap
<p>
tags around<?php esc_html_e( 'Some information may be filtered ...
- In
"### %s%S ###\n\n"
I noticed the capital%S
. - Sometimes
AS
and sometimesas
inforeach
loops.
Some other ideas:
- Replace
<a href="#system-information-table-of-contents">...
with<a href="#">...
to go to the top of the document? - Find a way to have the headings in view (hidden under the wp-admin-bar) when using named anchors
- Float the "Return to table of contents" links to the right (or left for rtl)
- Copy to clipboard without displaying the textarea? (might invovle cross-browser implementation issues?)
- Maybe the string can be simplified to "Private information has been filtered out from the list you are about to copy." ?
Then there's the accessibility to consider (like screen-reader-text etc)
I played with the new filter:
$external_info = apply_filters( 'debug_information', array() ); // Merge the core and external debug fields $info = array_merge( $external_info, $info );
with this demo:
add_filter( 'debug_information', 'warp_drive_debug_information' ); function warp_drive_debug_information ( $info ) { $info['warp-drive-info'] = array( 'label' => __( 'Warp Drive', 'startrek' ), 'description' => __( 'Data from the Enterprise Engine Room', 'startrek' ), 'show_count' => true, 'fields' => array( array( 'label' => __( 'Core status', 'startrek' ), 'value' => __( 'Stable', 'startrek' ) ), array( 'label' => __( 'Warp factor', 'startrek' ), 'value' => rand( 1, 10 ) ), ) ); return $info; }
and we can see in the screenshot here below how it's added on top of the core info.
Maybe some plugins will adjust the priority to stay on top?
I wonder if the external info should be added after the core info?
#31
@
7 years ago
Thanks for the links @zodiac1978
I just checked out the RS System Diagnostic plugin, it has some interesting info and features.
It showed additonally e.g.
- Permalink Structure
- Browser info for the current viewer
- Both "WP Memory Limit" and "WP Admin Memory Limit"
- Loaded PHP Extensions
- PHP Allow URL File Open
- PHP Allow URL File Incl
- ... more
I also checked out the BSI System Info plugins, it showed additionally e.g.:
- Default Timezone
- Operating System
- ... more
Regarding extending the System Information page:
- Should there be extra hooks for those who want to extend it, with features like download CSV button or send by email (like offered in the RS System Diagnostic plugin ). Currently it's not possible to add HTML, like a download button, in a section via the
debug_information
filter. If we allow extra HTML sections to be added after the info, e.g. via some action, then wouldn't it be nice to have that extra section in the ToC too?
- Should there be a private section, like private field?
This ticket was mentioned in Slack in #forums by mjassen. View the logs.
7 years ago
#33
@
7 years ago
in 39165.6.patch :
- Get active drop-ins
- Attempts to get server OS and architecture if possible
- MySQL client is included when possible
- Add check for function existing (being enabled) before using non-core functions, this avoids fatal errors
- Fixed the priority ordering, core sections come first
- Fixed text area indents on the first line
- Added permalink structure
- Added a
private
field to the section as well as single fields - Floated the back to ToC link based on language direction
I've also introduced a new section, *PHP Information*, this is essentially phpinfo()
being output within wp-admin. It includes a notice about the information contained in that page, it is intended for all those weird edge cases where there's that one specific setting you need to know about that wouldn't make sense to include in the debug output in the first place.
Basically that new "page" is there to not overload the normal debugging process with edge case scenario data, and instead allow individual developers to request information from specific fields in those rare cases.
As for some of the input provided:
Automatically copying text to the clipboard isn't possible, or rather, the API for it isn't there yet, those sites that do have this feature use a flash-polyfill where the clipboard is populated by a flash-video, it's a terrible hack and not worth it.
Linking to just #
takes you to the very top of the page, that's counter intuitive both for regular users, and screen reader users who might need it to jump between sections, it is much more logical to take you back to the table of contents list (although if I'm incorrect and there's a11y improvements to be had I'm all ears).
Extra hooks, I'm on the fence about, it's a debug page, the copy-field should really be enough for most situations, I'm not convinced the need to send them by email from within wp-admin is a legitimate purpose (and is more likely to get you picked up by a spam filter than anything else I'm afraid).
@
7 years ago
Add Filesystem permissions section and additional WP constants ABSPATH, WP_HOME and WP_SITEURL
#34
@
7 years ago
I like the idea with phpinfo()
, but some host providers and users, disable phpinfo()
with disable_functions
in php.ini
.
In that case$phpinfo_raw
contains:
Warning: phpinfo() has been disabled for security reasons in ...
39165.7.2.patch handles:
- if
phpinfo()
is disabled - if it's disabled, the phpinfo link in the header is removed
- if it's disabled, then
/wp-admin/info.php?phpinfo
shows the notice "The phpinfo() function is disabled.". - extra index checks on
$styles[1][0]}} and {{{$phpinfo[1][0]
.
Here I used a function_exists( 'phpinfo' )
check, but I wonder if that's enough or if we need to check inside: ini_get( 'disable_functions' )
? See e.g. here for discussions.
ps: I didn't noticed the last patch 39165.7.patch, so I didn't include it here.
#35
@
7 years ago
Good catch on the phpinfo()
being disabled in some scenarios @birgire, and good idea on the file system section @michalzuber
I've consolidated the two into a single patch (39165.8.patch) with some more verbiage and restructuring of some fields.
As for the function_exists()
, this accounts for things disabled via ini files I thought, although I could be wrong?
#36
@
7 years ago
@michalzuber
That sounds useful.
I think your new wp-filesystem
section should be all private
, or at least those fields that contain paths.
The strings 'writable'
and 'not writeable'
(maybe 'unwriteable'
?) would need to be translateable too.
@Clorith I just did a quick test on PHP 7.0.16 and it seems that function_exists()
will spot disabled phpinfo()
.
#37
@
7 years ago
Thank you. Nice job guys. Patch 39165.9-cron-jobs.patch contains my cron jobs listings try. I added cron jobs, because they are kinda hidden magic in WP for me and listing them is really revealing. Knowing what background actions are triggered.
While working on cron jobs I came across EMPTY_TRASH_DAYS
constant which I also added.
#38
@
7 years ago
I'm not yet convinced that this should be part of core.
Idea: Get https://wordpress.org/plugins/health-check/ back to life and include such a page into the plugin. The plugin would be some sort of an official plugin by the support team which gets actively promoted in the support forums.
If it turns out that this is super helpful we still can consider adding this to core, even if it's only in a form of "Install the Health Check plugin to get information about your installation" button that automatically installs the plugin. Take importers as an example.
Pinging @pento since he's the only active person listed as a contributor and committer. Could we get this on GitHub so others can contribute?
Other related plugins are https://wordpress.org/plugins/background-update-tester/ and https://wordpress.org/plugins/core-control/ from @dd32. Maybe these can also be part of Health Check? 🤔
#39
@
7 years ago
- Milestone 4.9 deleted
- Resolution set to wontfix
- Status changed from assigned to closed
I agree with @ocean90, this isn't right for core.
Putting it in the Health Check plugin is a good step, that was always the intention of that plugin.
I've put Health Check on GitHub, you should move the development work from this ticket over there. @Clorith, you can add collaborators to that repo as you see fit, and I've added you as a committer to the plugin on w.org. Go wild. :-)
#40
@
7 years ago
Agree with ocean90. Put the effort into a "all-in-one" plugin. Have a one-click install from a core page, that transforms into a real health check page once installed.
The core page should just check that plugins can be installed. If not, explain why and a way to rectify. Otherwise, just show the install button.
Because core should just do core things, useful for most users.
A plugin can be updated more frequently, even during a support session, a few hours or a day.
#41
follow-up:
↓ 43
@
7 years ago
As a supporter/moderator I disagree. As a plugin we can't rely on it being there. That's bad and costs time.
And the WP importer is a good example that a plugin is not the best way. Forgotten, not reliable, missing features, etc.
Because core should just do core things, useful for most users.
According to this argument there shouldn't be a page like options.php
and I don't think this is the case here BTW.
Frequent updates is an advantage, that's true, but the disadvantage of not included in core weighs much more IMHO.
#42
@
7 years ago
@zodiac1978 View it sort of like a feature project, we get to test the waters, a better way to see the needs. As @ocean90 mentioned we can always revisit the idea of core later, either by inclusion or as a single-click install.
It's not correct to directly compare it to the importer, the importer doesn't have a "team" behind it, while anything support related has a chunk of the community behind it to help move it forward and stay on top of things (fwiw: There are ideas floating around for the importer as well).
I would of course have preferred to have it as a part of core, so that everyone had a single location to reference for debug information, but I'm more than happy to approach it as a plugin first and take things from there as they appear (also gives us more flexibility with release and iterate, much better for ironing out what is needed for example).
#43
in reply to:
↑ 41
@
7 years ago
Replying to zodiac1978:
And the WP importer is a good example that a plugin is not the best way. Forgotten, not reliable, missing features, etc.
This can also apply to core features. I was also referring to the installation process not the plugins themselves.
#44
@
7 years ago
Many plugins implement a status page like WooCommerce to get stats about the system.
So this is an example that developers need to reimplement this things for their needs and improve the support or debug.
For me it will be better to have in the core and enabled (show) automatically when WP_DEBUG is turned on. In that way users doesn't need to install a plugin to use it only for fix a problem and developers need to ask to them to install another plugin (often there are systems where is not possible).
So developers can force the constant or with a new filter to show the panel and avoid another plugin.
After all is only showing systems stats that can be very helpful and we avoid to have a specific plugin for debugging when there are different and start a war about how is the best or is more standard for this feature.
Usually all the CMS or web software has a page included about internal stats like for permissions and WordPress is the only one without it.
Sure can be a feature plugin to develop with the plan to merge it in the future but not a separate plugin like for the importer that the use case are very few instead of a system page.
#45
@
7 years ago
This wasn't just for developers AFAIC. The idea is to avoid simple users going into wp-config.php as well to enable WP_DEBUG since most of the times the next question would be 'What is FTP?'
If you see it clearly from a support point of view a 'user' needs to do 2 clicks and return with the information needed to the developer that supports him/her without having to go back and forth to enable a siple informational screen. I see it the same with the debug.log.
Having it as a Plugin for the time being ( even forever as well ) makes a lot more sense as more things could be easily implemented or changed and pushed on production for anyone to have access to. Plus a plugin has pretty much the same steps on getting it installed & activated than having it built in as well, no major difference there, it's just 1 extra click and a search nothing more.
#46
@
7 years ago
Of course we can go this feature plugin route, but without the intention of merging it into core, this makes no sense at all. Why should I build *another* plugin to do that task? There are already plenty of them: https://core.trac.wordpress.org/ticket/39165#comment:29
And imagine this support question:
"I have problems with installing plugins." - "Okay, we need more details about your setup, please inst..." Oops. ;)
#47
@
7 years ago
Looking through the top 10 plugins by active installs, half contain similar functionality. Those that do contain it have a clear interest in actively improving user experiences inside WordPress. As opposed to those which do not like Akismet which is mostly an external service and Limit Login Attempts which has not been updated in 5 years.
It seems like this functionality is important to pro-actively solving user problems and improving the WordPress experience, which would support it being in core from my perspective. Further, as @zodiac1978 points out, users having problems installing plugins would be barred from using this functionality if it is external.
The less resistance between users having problems and solving those problems the better for WordPress.
Would a more thorough analysis of the usage across the plugin/theme directories help discussions?
#48
@
7 years ago
This ticket does need revisiting for sure. This shouldn't be closed and put forward for a plugin only. I think it'd be good to put it up as a plugin to develop it and then at some point merge it into Core. This would be a vital page of information for debugging issues with sites and would be useful to a whole range of users.
Need to get this reopened.
This ticket was mentioned in Slack in #forums by clorith. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by helen. View the logs.
7 years ago
#51
@
7 years ago
- Resolution wontfix deleted
- Status changed from closed to reopened
Reopening as this really ought to be in core. It should be as simple as a link in the footer that goes to this page. The entire health check plugin functionality could go in there. Simple mockup as made by @hedgefield coming right after this. I'll have someone on my team prepare a patch.
#52
@
7 years ago
@joostdevalk I actually want to disagree slightly, the debug information form the health check certainly should be in core (that's why we worked so hard on it for a while here).
The debug tab has gone mostly unchanged (it got the media information added recently, that's it), and is filterable for plugins and themes, this is what a info page in core should do.
The other features of the health check, automated tests and troubleshooting mode, are the kind of behaviors that truly should remain plugin territory. We've seen that the ability to do rapid enhancements to these sections have been a huge benefit to us and I would honestly prefer to keep those sections of the health check as they are, outside of core but in a "core plugin" so to speak.
#53
@
7 years ago
I'm mostly interested in the debug tab and the easy copy/paste functionality. I think that'd make support operators worldwide happy. Can you assist in making a patch for the above @Clorith ?
#54
@
7 years ago
Happy to get a patch sorted, should just be a matter of refreshing the current one here with the new media bit.
I must admit, I'm not entirely sold on adding a link to it, as opposed to the original plan of just having it be a directly accessible page for logged in users, but I'm open to hearing other opinions and pros/cons for this.
#55
@
7 years ago
A link in the footer, as unobtrusive as we've planned it, would allow support people to say "click on that link", which is a lot easier than saying "add this to your site's wp-admin dashboard URL".
#56
@
7 years ago
Happy to see the ticket is active again, but I guess I missed out something - is there a consensus that this should move forward within core and not go first through as a feature plugin, as was suggested before?
Update: ok found it on #core and didn't noticed at first the slack bot notice on this thread.
#57
@
7 years ago
Let's get this into trunk and see how it feels. I think a "Site Health" footer link and Tools > Site Health menu item is a good starting place to start experimenting with what info we want on the page, and how to present it.
#58
@
7 years ago
- Component changed from General to Administration
- Keywords needs-patch ui-feedback added
- Milestone set to 5.0
- Type changed from enhancement to task (blessed)
Milestoning for 5.0/trunk for now.
#59
@
7 years ago
39165.9.patch Introduces the footer link, basic styling to keep tables aligned for readability and an updated info.php
file that's brought in line with the Debug information
tab in the Health Check plugin at this time.
I'm still not sold on adding it to the side menu in wp-admin, it's not something we hope will need to be directly accessed in such a "grand" manner often, but the footer link works.
#60
@
7 years ago
I don't think it should be in the side menu. This looks good to me. @SergeyBiryukov could you have a look?
#63
@
7 years ago
This is coming along well, thanks @Clorith!
Some minor notes:
- Why is the active theme listed, but not the active plugins
- Minor detail: "Active theme" is written with a small "T", "Inactive Plugins" with a capital "P". I think title case should be used for all sections
- In this line:
( ! empty( $locale ) ? $original_locale : get_locale() )
—$original_locale
and$locale
are not defined. That's a remnant from the locale switching in the plugin. Just useget_locale()
. And perhaps rename the section to "Site Language" for clarification. $loopback = Health_Check_Loopback::can_perform_loopback();
— the class obviously doesn't exist in core, leading to a fatal error.- Typo: "this is information that may be considers private" -> "considered"
#64
@
7 years ago
Good spots, I was running my test with the plugin active and did not catch the missing class.
Active plugins should be listed, the active theme is more prominent because it's not something you often swap in and out like plugins I presume (the inspiration for that field came from the WooCommerce implementation of their own debug area, that is my understanding of that field at least).
Fixing title cases, relabling the site language bit and the local switch as well as typo are excellent.
As for the loopback checker, I'm wondering if it would make sense to take the loopback test that was put in for the theme and plugin editors and split it into a stand alone function for better reusability? It's currently part of wp_edit_theme_plugin_file()
.
#65
@
7 years ago
- Keywords has-patch added; needs-patch removed
39165.11.patch fixes what was mentioned in comment:63, I also opted to remove the loopback check. Although it can be handy to know if a loopback fails or not, the extended information on what is blocking them that the Health Check plugin can provide makes it a good field to be introduced by a plugin using the debug_information
filter in my opinion.
This ticket was mentioned in Slack in #meta by chanthaboune. View the logs.
7 years ago
This ticket was mentioned in Slack in #bbpress by casiepa. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by clorith. View the logs.
7 years ago
#70
@
7 years ago
One thing we found helpful in the Easy Digital Downloads "System Info" which we users supply us when they request support is not only including the current version of a plugin but also if there is a newer version available:
For example:
WP Crontrol: 1.5 (needs update - 1.6.2) WP Dictionary: 1.0 WP Fastest Cache: 0.8.7.3 (needs update - 0.8.7.6)
This quickly helps us identify possible conflicts as we can look through change logs when available.
#71
@
7 years ago
The Imagick Resource Limits field doesn't fit here because only scalar field values are supported:
// If Imagick is used as our editor, provide some more information about its limitations. if ( 'WP_Image_Editor_Imagick' === _wp_image_editor_choose() && isset( $imagick ) && $imagick instanceof Imagick ) { $limits = array( 'area' => ( defined( 'imagick::RESOURCETYPE_AREA' ) ? size_format( $imagick->getResourceLimit( imagick::RESOURCETYPE_AREA ) ) : 'Not Available' ), 'disk' => ( defined( 'imagick::RESOURCETYPE_DISK' ) ? $imagick->getResourceLimit( imagick::RESOURCETYPE_DISK ) : 'Not Available' ), 'file' => ( defined( 'imagick::RESOURCETYPE_FILE' ) ? $imagick->getResourceLimit( imagick::RESOURCETYPE_FILE ) : 'Not Available' ), 'map' => ( defined( 'imagick::RESOURCETYPE_MAP' ) ? size_format( $imagick->getResourceLimit( imagick::RESOURCETYPE_MAP ) ) : 'Not Available' ), 'memory' => ( defined( 'imagick::RESOURCETYPE_MEMORY' ) ? size_format( $imagick->getResourceLimit( imagick::RESOURCETYPE_MEMORY ) ) : 'Not Available' ), 'thread' => ( defined( 'imagick::RESOURCETYPE_THREAD' ) ? $imagick->getResourceLimit( imagick::RESOURCETYPE_THREAD ) : 'Not Available' ) ); $info['wp-media']['fields'][] = array( 'label' => __( 'Imagick Resource Limits' ), 'value' => $limits ); }
This results in PHP notices: "Array to string conversion".
'Not Available'
is also currently not translatable.
We would need a new line for each value here, but then I wonder if it is getting to much space, compared to other debug fields. So I removed it from the patch, in the meanwhile.
The patch 39165.12.diff:
- Fixes 134 Coding Standard issues.
- Removes the Imagick Resource Limits field, while it doesn't fit the current field value setup.
#72
@
7 years ago
The Imagick resource limits field is a requirement for troubleshooting media issues, so it's not an option to remove this, instead we should sort the error you saw. What part of it is causing a PHP notice, and what kind of setup are you on (do you have Imagick, and what PHP versions/Imagick versions)?
#73
@
7 years ago
The problem here is that the field values currently don't support arrays.
We have:
$info['wp-media']['fields'][] = array( 'label' => __( 'Imagick Resource Limits' ), 'value' => $limits //<-- a problematic array );
and there are two places where these field values are displayed with:
foreach ( $details['fields'] as $field ) { if ( isset( $field['private'] ) && true === $field['private'] ) { continue; } printf( "%s: %s\n", esc_html( $field['label'] ), esc_html( $field['value'] ) ); }
and
foreach ( $details['fields'] as $field ) { printf( '<tr><td>%s</td><td>%s</td></tr>', esc_html( $field['label'] ), esc_html( $field['value'] ) ); }
Two (three) options to fix that:
- New line for each
$limits
value. - Same line for each
$limits
value + translatable key label. Remove this field.
The "Same line" option looks favorable here. We could do a is_array()
check on the $field['value']
and loop through it to display each array value, making sure the 'Not Available' is translatable and also the array key labels. These arrays could be displayed as a list, a sub-table or even as <span>
separated words?
#74
@
7 years ago
Ahh yes, we've been running this in the health check plugin for a bit, and I did add support for this there ahead of time so plugins could utilize it if they wanted.
foreach ( $details['fields'] as $field ) { if ( is_array( $field['value'] ) ) { $values = ''; foreach ( $field['value'] as $name => $value ) { $values .= sprintf( '<li>%s: %s</li>', esc_html( $name ), esc_html( $value ) ); } } else { $values = esc_html( $field['value'] ); } printf( '<tr><td>%s</td><td>%s</td></tr>', esc_html( $field['label'] ), $values ); }
#75
@
7 years ago
yebb, that looks good and what I had in mind ;-)
I think we should consider this kind of structure for field value arrays:
array( 'label' => __( 'Some label' ), 'value' => 'Some value', );
so the label can be translatable.
#76
@
7 years ago
The patch 39165.13.diff:
- Adds a "sub-fields" support, i.e. field value arrays with structure like:
$info['example']['fields'][] = array( 'label' => __( 'Label' ), 'value' => array( array( 'label' => __( 'Sub Label 1' ), 'value' => 'Sub Value 1', ), array( 'label' => __( 'Sub Label 2' ), 'value' => 'Sub Value 2', ), ), );
- Adjusts the
$info['wp-media']['fields']
array accordingly. - Makes
'Not Available'
translatable.
To consider:
- Adding a
isset()
checks for label and value array items for both fields and sub-fields. - Using a descriptive list
<dl>
for subfields.
#77
@
7 years ago
@Clorith , Would it be plus to add the Latest Version
information of Themes / Plugins and the extra call to check for updates when you enter the debug page here as well?
#78
@
7 years ago
Yeah, definitely (it was discussed a few replies ago). Would absolutely be a plus if update triggers ran here as well as you mentioned.
#79
@
7 years ago
39165.14.diff triggers the wp_version_check()
wp_update_plugins()
wp_update_themes()
only in the debug view and adds the update information on themes & plugins if needed
#80
@
7 years ago
39165.15.diff is just a follow up on the idea of 39165.14.diff .
There are some plugins that disable the Update notices etc, this way there is an extra note when there's a core update.
#81
@
7 years ago
The patch in 39165.16.diff has few other suggestions that:
- Makes these strings translatable:
-
$imagick_version = 'Imagick not available';
-
$gs = ( ! empty( $gs ) ? $gs : 'Not available' );
-
'value' => ( is_array( $gd ) ? $gd['GD Version'] : 'GD not available' ),
-
- Moves inline CSS style to
debug.css
:style="display: block; width: 100%; text-align: <?php echo ( is_rtl() ? 'left' : 'right' ); ?>"
debug-rtl.css
would take care of thetext-align left;
case. - Uses
<div>
instead of<span style="display: block; width: 100%;">
- Adds numbered placeholders for:
'<a href="#%s" class="debug-toc">%s</a>'
"\n\t %s: %s"
"### %s%s ###\n\n"
'<h2 id="%s">%s%s</h2>'
'<li>%s: %s</li>'
'<tr><td>%s</td><td>%s</td></tr>'
"%s: %s\n"
'<a href="%s">%s</a>'
(inadmin-footer.php
)
- Changes
$item
to$subfield
. - Checks if the sub-field's label and value are set:
if( isset( $subfield['label'], $subfield['value'] ) )
- Adds a full stop (dot) for:
// Trigger all update checks
'Show debug info'
(inadmin-footer.php
)
- Uses tab instead of space indentation for the part in
admin-footer.php
. - Removes a space in array key:
$info[ 'wp-active-theme']['fields']
- Adds a span tag around the counters:
' <span class="count">(%d)</span>'
- Capitalizes all words in:
'System information'
'Other themes'
'Media handling'
'WordPress constants'
'Filesystem permissions'
'Permalink structure'
'Not available'
'Server architecture'
'Server settings'
'PHP time limit'
'PHP memory limit'
'PHP max input variables'
'Max input time'
'Upload max filesize'
'PHP post max size'
'SUHOSIN installed'
'Server version'
'Client version'
'Database user'
'Database host'
'Database name'
'Database prefix'
'Author website'
'Parent theme'
'Supported theme features'
'The Must Use Plugins directory'
'Not writable'
- Re-uses the existing strings from
wp-admin/options-general.php
:'Home URL'
->'Site Address (URL)'
'Site URL'
->'WordPress Address (URL)'
- Wraps the javascript into an anonymous function:
( function( $ ) { ... })( jQuery )
. - Adjusts the javascript according to jshint.
- Avoids the
onclick
attribute and moves it to the script tag instead:onclick="document.getElementById('system-information-copy-wrapper').style.display = 'block'; this.style.display = 'none';"
onclick="document.getElementById('system-information-copy-field').select(); document.execCommand( 'copy' );"
- Uses
show()/hide()
in jQuery. - Uses
on( 'click', ... )
instead of the shortcutclick()
. Wasn't sure though what's preferred. - Adds JS DocBlocks.
- Changes
?> <?php } ?> <?php } ?>
to} } ?>
.
To consider:
- The ticket #40037 Add ability to ask wpdb for full db server info introduces the method
wpdb::db_server_info()
that could be useful here. - Create a
debug.js
file for the javascript? - Should more translatable strings end in a dot (.) ?
- Should we move the parentheses out of this translatable string:
'( Latest version: %s )'
? - Fix parentheses display for RTL languages. (See e.g. #43476)
- Should we use:
'Not Available'
instead of'Imagick not available'
'Not Available'
instead of'GD not available'
- Have the text
'Show debug info.'
filterable, like for other text in the footer?
#82
@
7 years ago
Are the DB details really actually useful to debugging anyones sites? Keeping mind that these are likely to be copy-pasted and/or sent to plugin developers as part of debugging:
Database user
Database host
Database name
Database prefix
Assuming someone can access the page, it's fair to assume that these values are correct, and broadcasting them isn't likely to be useful to anyone.
I'd be curious to hear if anyone has any real reason of why they're useful (knowing that the prefix isn't wp_
can be useful sometimes with badly coded plugins, but that's about it IMHO)
#83
@
7 years ago
It's based off what you'd expect a debug page to include really, the prefix is for sharing (you'll notice the copy-field excludes the other entries for the reason you mentioned, they shouldn't be posted on forums for example).
The host is used to determine the DB location, if you've changed hosts this is valuable and the target audience here is the average user who may not be comfortable looking directly at a config file (many hosts will do the migration for you so you never even consider the hostname/db name/username).
#84
@
6 years ago
Following up on this ticket, what do we still need here (besides possibly a refresh of the patch) ?
#85
@
6 years ago
It would be great to have the current state in trunk for some time, then we can iterate from there.
This ticket was mentioned in Slack in #design by karmatosed. View the logs.
6 years ago
#87
@
6 years ago
This is a really helpful issue! We just discussed in the #design team Triage meeting and wanted to share our feedback:
- The latest design showing the system details works well. Assuming there's a copy button at the bottom? If so that should be fine. As for suggestions we could probably figure out what happens when text goes too long, does it wrap? And what would this look like on mobile?
- We could probably remove the notification at the top that encourages copy+pasting: https://core.trac.wordpress.org/attachment/ticket/39165/filter-example.jpg
- The link to the health check in the footer works well: https://core.trac.wordpress.org/attachment/ticket/39165/Health%20Check%20footer.png
This ticket was mentioned in Slack in #core by clorith. View the logs.
6 years ago
This ticket was mentioned in Slack in #core by codebard. View the logs.
6 years ago
This ticket was mentioned in Slack in #design by karmatosed. View the logs.
6 years ago
#95
@
6 years ago
So, there's two screenshots.
The first one, simplified.png is if the other features aren't done in time, we can do it in stages instead, making this section land for 5.2
The second screenshot, full-version.png is essentially the same, but it includes navigation between the two pages, and the status indicator that ties in with the site status page.
#97
@
6 years ago
- Milestone 5.2 deleted
- Resolution set to duplicate
- Status changed from reopened to closed
I'm closing this as a duplicate of #46573 at this time, the other ticket has some traction at this time, and tackles more than just the debug screen. It has been stylized along with the site health information, and adds various new data for debugging purposes and making it much more friendly for international use.
Chat that inspired this ticket