WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 5 months ago

Last modified 5 months ago

#42697 closed defect (bug) (fixed)

Regression - Missing Languages update section

Reported by: selven99 Owned by: flixos90
Milestone: 4.9.3 Priority: normal
Severity: normal Version: 4.9
Component: Role/Capability Keywords: has-patch
Focuses: Cc:

Description

Hello,

Since I updated my websites to Wordpress 4.9, Languages Update section disappears in Dashboard > Updates section!

After some debug, I found the issue with the new capabilities added in this version.

https://github.com/WordPress/WordPress/blob/4.9-branch/wp-includes/capabilities.php#L443

Here:

<?php
    if ( ! wp_can_install_language_pack() ) {
        $caps[] = 'do_not_allow';
    } elseif ( is_multisite() && ! is_super_admin( $user_id ) ) {
        $caps[] = 'do_not_allow';
    } else {
        $caps[] = 'install_languages';
    }

The first line will check if FTP (or SFTP/SSH) details are corrects and if wordpress can access to filesystem. But, creditentials are not provided in this step, we can fill them only after click to "Update translations" button (that is missing...).

So, I think this implementation is incorrect and should be:

<?php
    if ( ! wp_is_file_mod_allowed( 'can_install_language_pack' ) ) {
        $caps[] = 'do_not_allow';
    } elseif ( is_multisite() && ! is_super_admin( $user_id ) ) {
        $caps[] = 'do_not_allow';
    } else {
        $caps[] = 'install_languages';
    }

What do you think about this?

Thanks! :-)

Attachments (1)

42697.diff (4.0 KB) - added by flixos90 7 months ago.

Download all attachments as: .zip

Change History (15)

#1 @dd32
7 months ago

  • Milestone changed from Awaiting Review to 4.9.2

Hey @selven99 and welcome to Trac.

Thanks for reporting this, for reference, the changes related to this were introduced in [41268] via #39677. Pinging @flixos90 for investigation.

Marking as 4.9.2 for tracking purposes only.

#3 @dd32
7 months ago

#42767 was marked as a duplicate.

#4 @flixos90
7 months ago

  • Owner set to flixos90
  • Status changed from new to assigned

@flixos90
7 months ago

#5 @flixos90
7 months ago

  • Component changed from Upgrade/Install to Role/Capability
  • Keywords has-patch added

Thanks @selven99 for the ticket and also suggestion to fix. I applied it in 42697.diff, including related adjustments:

  • By using wp_is_file_mod_allowed(), we make the capability check for install_languages less restrictive. It was indeed wrong before, as a user should still be able to have the capability, even when language packs cannot be automatically installed. The automatic installation part is separate from that now.
  • Another benefit from that change is that we don't need to load the related admin files on the fly in the frontend.
  • The "downside" is that of several conditionals now need to check for both the capability and whether language packs can be installed without asking for credentials. However, I'd only consider this a downside because of more code. The two checks actually need to be separate things, for the above reasons.

#6 @obenland
7 months ago

The "downside" is that of several conditionals now need to check for both the capability and whether language packs can be installed without asking for credentials. However, I'd only consider this a downside because of more code. The two checks actually need to be separate things, for the above reasons.

Would this be something that could be moved into map_meta_cap()?

#7 @dd32
6 months ago

Apologies for the delay in reviewing @flixos90 Looking at 42697.diff the capability change looks correct, however I question the changes to show_available_translations.

It seems to me that this is probably expected as-is, if it can't install the language it'd do it on the next time it does have access. The user-experience of that would be pretty bad though - They'd either a) Select it and it not take immediate effect or b) not be able to change languages to a non-installed one at all.

For this ticket, it seems that a) is probably okay, although not great. Further enhancement is needed for it to instead trigger the FTP credentials request to install the language selected IMHO.

#8 follow-up: @flixos90
6 months ago

@obenland It was in there before, but that’s what ended up not working, as you should generally be able to install languages even if you need to provide FTP credentials for it.

@dd32 The original behavior was using wp_can_install_language_pack() in show_available_translations (see [41268]). While I agree with you that this is far from optimal, I think we should add it back in for now to fix the regression, with further changes being part of a new enhancement ticket.

#9 in reply to: ↑ 8 @dd32
6 months ago

Replying to flixos90:

The original behavior was using wp_can_install_language_pack() in show_available_translations (see [41268]). While I agree with you that this is far from optimal, I think we should add it back in for now to fix the regression, with further changes being part of a new enhancement ticket.

Got it, that makes sense to me then!

#10 @dd32
5 months ago

  • Milestone changed from 4.9.2 to 4.9.3

Bumping to 4.9.3 due to 4.9.2s release

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


5 months ago

#12 @SergeyBiryukov
5 months ago

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

In 42606:

Role/Capability: Make install_languages capability check less restrictive.

A user should still be able to have the capability, even when language packs cannot be automatically installed. The automatic installation part is separate from that now.

Props flixos90.
Fixes #42697.

#13 @SergeyBiryukov
5 months ago

In 42607:

Role/Capability: Make install_languages capability check less restrictive.

A user should still be able to have the capability, even when language packs cannot be automatically installed. The automatic installation part is separate from that now.

Props flixos90.
Merges [42606] to the 4.9 branch.
Fixes #42697.

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


5 months ago

Note: See TracTickets for help on using tickets.