This repository has been archived by the owner on Mar 9, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 4
WP-r56622: Add sys_get_temp_dir()
to open_basedir
tests
#218
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…r` tests. In PHPUnit 10.3.5, 9.6.13 and 8.5.34, the child processes used for process isolation now use temporary files to communicate their result to the parent process. This caused a failure in some tests that set the `open_basedir` PHP directive to a value that did not include `sys_get_temp_dir()`. This adds `sys_get_temp_dir()` to the `open_basedir` value set by the tests to ensure that permission is still granted for the temporary directory. PHPUnit uses `sys_get_temp_dir()`. To ensure the result is the same, Core's `get_temp_dir()` function is not used. References: - sebastianbergmann/phpunit#5356 WP:Props desrosj, mukesh27, SergeyBiryukov, costdev. Fixes https://core.trac.wordpress.org/ticket/59394. --- Merges https://core.trac.wordpress.org/changeset/56622 / WordPress/wordpress-develop@7d96189ba1 to ClassicPress.
xxsimoxx
approved these changes
Sep 20, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two errors are fixed in my local install.
The Unit test errors do appear to be linked with the release of PHPUnit 9.6.13 yesterday. Locally I had PHPUnit 9.6.12 and the tests were fine. updating to 9.6.13 with Will merge this and then |
mattyrob
added a commit
that referenced
this pull request
Sep 21, 2023
* Update nav-menu.js Improve feedback for accessibility when moving items in nav menu * Use localized strings in nav-menu.js * Add new localized strings to nav-menus.php Adds new localized strings to `nav-menus.php` to make them available in `nav-menu.js` * Renovate[bot]: Update dependency rollup to v3.29.2 (#211) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Renovate[bot]: Update dependency grunt-contrib-qunit to v8.0.1 (#212) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * JS Coding standards fixes * Update common.css Removes CSS that's no longer needed because of previous commits, and adds new CSS to make the nav menu page more accessible. * Renovate[bot]: Update dependency postcss to v8.4.30 (#215) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update common.css Changes `focus-within` to use `outline` instead of `border` * Update nav-menu.js After some testing, I have found that merely adding `setTimeout` in itself creates enough of a delay to enable the correct message to be generated. There is no need to set a value greater than 0. * WP-r56622: Add `sys_get_temp_dir()` to `open_basedir` tests. (#218) In PHPUnit 10.3.5, 9.6.13 and 8.5.34, the child processes used for process isolation now use temporary files to communicate their result to the parent process. This caused a failure in some tests that set the `open_basedir` PHP directive to a value that did not include `sys_get_temp_dir()`. This adds `sys_get_temp_dir()` to the `open_basedir` value set by the tests to ensure that permission is still granted for the temporary directory. PHPUnit uses `sys_get_temp_dir()`. To ensure the result is the same, Core's `get_temp_dir()` function is not used. References: - sebastianbergmann/phpunit#5356 WP:Props desrosj, mukesh27, SergeyBiryukov, costdev. Fixes https://core.trac.wordpress.org/ticket/59394. --- Merges https://core.trac.wordpress.org/changeset/56622 / WordPress/wordpress-develop@7d96189ba1 to ClassicPress. Co-authored-by: Colin Stewart <[email protected]> * Renovate[bot]: Update dependency node to v18.18.0 (#216) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Update nav-menu.js Improve feedback for accessibility when moving items in nav menu * Use localized strings in nav-menu.js * Add new localized strings to nav-menus.php Adds new localized strings to `nav-menus.php` to make them available in `nav-menu.js` * JS Coding standards fixes * Update common.css Removes CSS that's no longer needed because of previous commits, and adds new CSS to make the nav menu page more accessible. * Update common.css Changes `focus-within` to use `outline` instead of `border` * Update nav-menu.js After some testing, I have found that merely adding `setTimeout` in itself creates enough of a delay to enable the correct message to be generated. There is no need to set a value greater than 0. --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: mattyrob <[email protected]> Co-authored-by: Colin Stewart <[email protected]>
mattyrob
added a commit
that referenced
this pull request
Oct 5, 2023
* Adds details and summary elements to post metaboxes and dashboard widgets This enhances accessibility for post metaboxes and dashboard widgets and enables them to be toggled open and closed without JavaScript. * Update edit.css Ensures that post metaboxes and dashboard widgets look the same as before the use of details and summary elements. * Update common.css Enures that post metaboxes and dashboard widgets look the same as before the use of details and summary elements. * Update postbox.js Ensures that open/closed state of post metaboxes and dashboard widgets is preserved across page loads. Before it was based on the presence or otherwise of the class "closed"; now it is dependent on the presence or otherwise of the attribute "open". * Update sortable.js Sortable widgets in the CP backend currently rely on jQuery UI. While it would be nice to replace that with something more modern (like SortableJS), that has to wait for another day. In the meantime, we need a fix to address the fact that the HTML5 elements `details` and `summary` were introduced after development ceased on jQuery UI. This fix simply avoids the presence of an ugly artefact when dragging and dropping a `details` element. * WP-r56622: Add `sys_get_temp_dir()` to `open_basedir` tests. (#218) In PHPUnit 10.3.5, 9.6.13 and 8.5.34, the child processes used for process isolation now use temporary files to communicate their result to the parent process. This caused a failure in some tests that set the `open_basedir` PHP directive to a value that did not include `sys_get_temp_dir()`. This adds `sys_get_temp_dir()` to the `open_basedir` value set by the tests to ensure that permission is still granted for the temporary directory. PHPUnit uses `sys_get_temp_dir()`. To ensure the result is the same, Core's `get_temp_dir()` function is not used. References: - sebastianbergmann/phpunit#5356 WP:Props desrosj, mukesh27, SergeyBiryukov, costdev. Fixes https://core.trac.wordpress.org/ticket/59394. --- Merges https://core.trac.wordpress.org/changeset/56622 / WordPress/wordpress-develop@7d96189ba1 to ClassicPress. Co-authored-by: Colin Stewart <[email protected]> * Renovate[bot]: Update dependency node to v18.18.0 (#216) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Adds details and summary elements to post metaboxes and dashboard widgets This enhances accessibility for post metaboxes and dashboard widgets and enables them to be toggled open and closed without JavaScript. * Update edit.css Ensures that post metaboxes and dashboard widgets look the same as before the use of details and summary elements. * Update common.css Enures that post metaboxes and dashboard widgets look the same as before the use of details and summary elements. * Update postbox.js Ensures that open/closed state of post metaboxes and dashboard widgets is preserved across page loads. Before it was based on the presence or otherwise of the class "closed"; now it is dependent on the presence or otherwise of the attribute "open". * Update sortable.js Sortable widgets in the CP backend currently rely on jQuery UI. While it would be nice to replace that with something more modern (like SortableJS), that has to wait for another day. In the meantime, we need a fix to address the fact that the HTML5 elements `details` and `summary` were introduced after development ceased on jQuery UI. This fix simply avoids the presence of an ugly artefact when dragging and dropping a `details` element. * PHP Coding standards * JS Coding Standards * Update common.js to address Safari's failure to respect effect of closure of details element Addresses the focusable issue mentioned in this comment: #217 (comment) This commit applies the `inert` attribute to elements within a closed `details` element. This appears to be supported by both desktop and mobile versions of Safari: see https://caniuse.com/?search=inert * Update common.css to replace details arrows via CSS so that they can also have a pointer cursor. * Update common.css to improve style of arrows * JS syntax corrections * Update common.css to make `details` marker bigger and remove the default in Safari. * Compliant CSS comment * Suggested Safari fix * Update common.css for summary header and custom marker * Update template.php to add back `h2` and `h3` tags. This responds to feedback at #217 (comment) This should also resolve the issue where clicking on the pointer icon still enabled the widget to be dragged. Dragging is now possible only where the draggable icon appears. * Update template.php to move a class to avoid causing problems on the admin menus page. * Update common.css to modify styling on admin menus page. The changes made elsewhere in the PR have an impact on the visual styling of the admin menus page. For reasons long forgotten, and as noted in this file already, that page works differently from other admin pages that use widgets. So some modifications are necessary to ensure the menus page continues to look as expected. * Update template.php to remove tabindex and enqueuing of deleted script. * Fix whitespace in wp-admin/includes/template.php --------- Co-authored-by: Matt Robinson <[email protected]> Co-authored-by: Colin Stewart <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
WP-r56622: Build/Test Tools: Add
sys_get_temp_dir()
toopen_basedir
tests.In PHPUnit 10.3.5, 9.6.13 and 8.5.34, the child processes used for process isolation now use temporary files to communicate their result to the parent process.
This caused a failure in some tests that set the
open_basedir
PHP directive to a value that did not includesys_get_temp_dir()
.This adds
sys_get_temp_dir()
to theopen_basedir
value set by the tests to ensure that permission is still granted for the temporary directory.PHPUnit uses
sys_get_temp_dir()
. To ensure the result is the same, Core'sget_temp_dir()
function is not used.References:
WP:Props desrosj, mukesh27, SergeyBiryukov, costdev. Fixes https://core.trac.wordpress.org/ticket/59394.
Merges https://core.trac.wordpress.org/changeset/56622 / WordPress/wordpress-develop@7d96189ba1 to ClassicPress.
Motivation and context
Hopefully, this will fix 2 failing PHPUnit tests in core.
How has this been tested?
Upstream backport and unit tests will run on PR.
Screenshots
N/A
Types of changes