Make WordPress Core

Opened 9 months ago

Closed 6 months ago

Last modified 6 months ago

#60719 closed enhancement (fixed)

Add fonts upload location to Site Health report

Reported by: swissspidy's profile swissspidy Owned by: audrasjb's profile audrasjb
Milestone: 6.6 Priority: normal
Severity: normal Version:
Component: Site Health Keywords: good-first-bug has-patch has-screenshots has-testing-info
Focuses: Cc:

Description

Just like Site Health lists the paths to the plugins/themes/uploads directories, we should list the path to the fonts directory (and its size) there as well.

Attachments (9)

Screenshot.jpg (154.3 KB) - added by krupajnanda 9 months ago.
Screenshot 2024-05-10 at 10.51.27 AM.png (152.9 KB) - added by naeemhaque 7 months ago.
Test report #60719 by @naeemhaque
Screenshot at May 10 10-52-59.png (254.9 KB) - added by iamfarhan09 7 months ago.
Ticket #60719 @iamfarhan09
Test report #60719 by @ imasikur22.png (161.5 KB) - added by imasikur22 7 months ago.
Test report #60719 by @ imasikur22
Screenshot 2024-05-10 at 11.38.40 AM.png (149.5 KB) - added by tofajjal02 7 months ago.
Ticket #60719 @tofajjal02
Test report #60719.png (254.8 KB) - added by prottoysarkar 7 months ago.
Ticket #60719 @prottoysarkar
Screenshot_5.png (24.9 KB) - added by imranhasanraaz 7 months ago.
Test Report #60719 - Nazmul Hosen.png (30.3 KB) - added by nazmul111 7 months ago.
After Applying this patch for #60719
Capture d’écran 2024-05-28 à 00.11.19.png (202.0 KB) - added by audrasjb 7 months ago.
Works fine

Download all attachments as: .zip

Change History (38)

#1 @swissspidy
9 months ago

There could also be a general check whether wp-content itself is writable or not.

Edit: this already exists!

Last edited 9 months ago by swissspidy (previous) (diff)

This ticket was mentioned in PR #6260 on WordPress/wordpress-develop by @andr3ribeiro.


9 months ago
#2

  • Keywords has-patch added; needs-patch removed

Added the font directory in the array for site health table iteration

This ticket was mentioned in Slack in #core-test by krupajnanda. View the logs.


9 months ago

#4 @krupajnanda
9 months ago

  • Keywords has-screenshots has-testing-info added

I tested changes with mentioned GH PR and it shows the font dir path and size to Site Health.

Test Report

Description

This report validates whether the indicated patch works as expected.

PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.2.15
  • Server: nginx/1.25.3
  • Database: mysqli (Server: 8.0.36 / Client: mysqlnd 8.2.15)
  • Browser: Chrome 123.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Actual Results

  1. ✅ Added changes are working as expected with given PR.

Additional Notes

  • Any additional details worth mentioning.

Supplemental Artifacts

Add as Attachment

This ticket was mentioned in Slack in #core by andr3ribeiro. View the logs.


9 months ago

@ironprogrammer commented on PR #6260:


8 months ago
#6

Thanks for the PR, @andr3ribeiro! Perhaps a good addition to this would be to include the fonts dir "is writable" check found on the Filesystem Permissions tab. This work is something we could probably get merged early this cycle.

#7 @iamfarhan09
7 months ago

Bug Report

Description

According to this report, the patch indicated works as expected.

PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 7.4.33
  • Server: nginx/1.25.1
  • Database: mysqli (Server: 8.0.37 / Client: mysqlnd 7.4.33)
  • Browser: Chrome 124.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Actual Results

  1. ✅ I have tested the added changes with the given PR and they are working as expected.

Attachment Screenshot at May 10 10-52-59.png​ added

Last edited 7 months ago by iamfarhan09 (previous) (diff)

#8 @iamfarhan09
7 months ago

Last edited 7 months ago by iamfarhan09 (previous) (diff)

#9 @naeemhaque
7 months ago

Bug Report

Description

This report validates whether the indicated patch works as expected.
PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 7.4.33
  • Server: nginx/1.23.2
  • Database: mysqli (Server: 8.0.36 / Client: mysqlnd 7.4.33)
  • Browser: Chrome 124.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Steps to Reproduce

Actual Results

  1. ✅ The PR is working like expected result.
Last edited 7 months ago by naeemhaque (previous) (diff)

@naeemhaque
7 months ago

Test report #60719 by @naeemhaque

@iamfarhan09
7 months ago

Ticket #60719 @iamfarhan09

#10 @imasikur22
7 months ago

Bug Report

Description

According to this report, the patch indicated works as expected.
PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.2.18
  • Server: nginx/1.25.5
  • Database: mysqli (Server: 8.0.37 / Client: mysqlnd 8.2.18)
  • Browser: Chrome 124.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Expected Results

  1. ✅ The PR is working like expected result.

Additional Notes

  • The result is working fine but the status is showing "Loading..."

@imasikur22
7 months ago

Test report #60719 by @ imasikur22

#11 @tofajjal02
7 months ago

Bug Report

Description

According to this report, the patch indicated works as expected.

PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.2.18
  • Server: nginx/1.25.5
  • Database: mysqli (Server: 8.0.37 / Client: mysqlnd 8.2.18)
  • Browser: Chrome 124.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Actual Results

  1. ✅ The pr working is final as a expected result

#12 @sarkarripon
7 months ago

Bug Report

Description

According to this report, the patch indicated works as expected.
PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.2.18
  • Server: nginx/1.25.5
  • Database: mysqli (Server: 8.0.37 / Client: mysqlnd 8.2.18)
  • Browser: Chrome 124.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Actual Results

  1. ✅ PR worked fine from my end

Supplemental Artifacts

Add Inline: https://cln.sh/lZm38W0fsHWzcfNQPTf5

#13 @prottoysarkar
7 months ago

Bug Report

Description

According to this report, the patch described works as expected.

PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

WordPress: 6.6-alpha-57778-src
PHP: 7.4.33
Server: nginx/1.25.1
Database: mysqli (Server: 8.0.37 / Client: mysqlnd 7.4.33)
Browser: Chrome 124.0.0.0
OS: macOS
Theme: Twenty Twenty-Four 1.0
MU Plugins: None activated
Plugins:
Test Reports 1.1.0

Actual Results

✅ I tested the changes with the given PR and they work as expected.

@prottoysarkar
7 months ago

Ticket #60719 @prottoysarkar

This ticket was mentioned in Slack in #core-test by zunaid321. View the logs.


7 months ago

#15 @zunaid321
7 months ago

Bug Report

Description

This report validates whether the indicated patch works as expected.
PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.2.12
  • Server: nginx/1.25.3
  • Database: mysqli (Server: 8.0.36 / Client: mysqlnd 8.2.12)
  • Browser: Chrome 124.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Expected Results

  1. ✅ PR worked fine but the status was stuck at "loading".

Supplemental Artifacts

Screenshot: https://i.ibb.co/WyTdskf/2024-05-13-14-13.png

This ticket was mentioned in Slack in #core-test by zunaid321. View the logs.


7 months ago

#17 @imranhasanraaz
7 months ago

Test Report
Description
According to this report, the patch described works as expected.
PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.2.19
  • Server: nginx/1.25.5
  • Database: mysqli (Server: 8.0.37 / Client: mysqlnd 8.2.19)
  • Browser: Chrome 124.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-One 2.2
  • MU Plugins: None activated
  • Plugins:
    • Hello Dolly 1.7.2
    • Test Reports 1.1.0

Actual Results
✅ I tested the changes with the given PR and they work as expected.
Screenshot: https://prnt.sc/BA7iCJdxi4tt

Last edited 7 months ago by imranhasanraaz (previous) (diff)

#18 @rajinsharwar
7 months ago

  • Keywords commit added

Tested changes with mentioned the PR and it shows the font dir path and size to Site Health.
PR tested: https://github.com/WordPress/wordpress-develop/pull/6260
Environment
WordPress: 6.6
PHP: 8.1.23
Server: nginx
Database: mysqli
OS: Windows

✅ The font directory is showing correctly.

https://img001.prntscr.com/file/img001/KR080oABRAKkPy9R4jQ3ug.png

As this ticket has an adequate number of test reports, marking this for 'commit'.

#19 @nazmul111
7 months ago

Test Report

Description

This report validates whether the indicated patch works as expected.
PR tested: https://github.com/WordPress/wordpress-develop/pull/6260

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.2.19
  • Server: nginx/1.25.5
  • Database: mysqli (Server: 8.0.37 / Client: mysqlnd 8.2.19)
  • Browser: Chrome 125.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Expected Results

  1. ✅ PR worked fine but the status was stuck at "loading".
Last edited 7 months ago by nazmul111 (previous) (diff)

@nazmul111
7 months ago

After Applying this patch for #60719

#20 @audrasjb
7 months ago

  • Owner set to audrasjb
  • Status changed from new to accepted

Thanks everyone for testing 😅

The PR works fine on my side as well.
Self assigning for commit.

@audrasjb commented on PR #6260:


6 months ago
#21

I committed the changes suggested by @peterwilsoncc directly to the PR.

#22 @audrasjb
6 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 58299:

Site Health: Add fonts upload location to Site Health report.

Just like Site Health lists the paths to the plugins/themes/uploads directories, this changeset adds the path to the fonts directory (and its size).

Props swissspidy, andr3ribeiro, krupajnanda, ironprogrammer, iamfarhan09, naeemhaque, imasikur22, tofajjal02, sarkarripon, prottoysarkar, zunaid321, imranhasanraaz, rajinsharwar, nazmul111, audrasjb, peterwilsoncc.
Fixes #60719.

#24 follow-up: @audrasjb
6 months ago

  • Keywords commit removed

@ironprogrammer I committed the current PR without the change you suggested, to make sure this enhancement can ship in time for 6.6 beta 1. We can either reopen this ticket for a follow-up patch or open a new one.

#25 in reply to: ↑ 24 @ironprogrammer
6 months ago

Replying to audrasjb:

@ironprogrammer I committed the current PR without the change you suggested, to make sure this enhancement can ship in time for 6.6 beta 1. We can either reopen this ticket for a follow-up patch or open a new one.

Thanks, @audrasjb! I'll create a PR with the follow-up patch shortly (I've got it stashed somewhere).

#27 @audrasjb
6 months ago

In 58310:

Site Health: Add a wp_is_writable() for fonts directory.

This changeset adds a check in Site Health to display whether the fonts directory is writable or not.

Follow-up to [58299].

Props ironprogrammer.
Fixes #60719.

#29 @desrosj
6 months ago

In 58408:

Coding Standards: Apply changes after running composer format.

This applies several formatting related changes made while running composer format.

Follow up to [55720], [58171], [58271], [58282], [58283], [58292], [58299], [58303], [58332].
See #51857, #60719, #60895, #61021, #61118, #61228, #61276, #61324.

Note: See TracTickets for help on using tickets.