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.
System_modules_submit() contains the following code that should be put in locale_modules_enabled():
// Notify locale module about module changes, so translations can be
// imported. This might start a batch, and only return to the redirect
// path after that.
module_invoke('locale', 'system_update', $new_modules);
Comment | File | Size | Author |
---|---|---|---|
#25 | 545518-follow-up-fix.patch | 670 bytes | bfroehle |
#13 | drupal.locale-import.13.patch | 2.63 KB | sun |
#4 | drupal.locale-import.4.patch | 2.71 KB | sun |
#1 | locale_modules_enabled_00.patch | 1.76 KB | Xano |
Comments
Comment #1
XanoComment #2
XanoComment #4
sunGood catch!
Comment #5
sunAny feedback?
Comment #6
sun#4: drupal.locale-import.4.patch queued for re-testing.
Comment #7
sunComment #8
sun#4: drupal.locale-import.4.patch queued for re-testing.
Comment #9
sunAlthough badly needed, this is D8 material according to the rules (I had to learn today). It may be backported at a later point in time (though that's unlikely).
Comment #10
sunHm. This doesn't really affect any functionality and merely moves code where it belongs.
Comment #11
sun#4: drupal.locale-import.4.patch queued for re-testing.
Comment #13
sunRe-rolled against HEAD.
Comment #14
XanoLooks good to me, but I haven't got a D7 install here at the moment, so I cannot yet confirm whether it works or not.
Comment #15
sunFWIW, this functionality is covered by tests. But happy to wait for a manual test, too.
Comment #16
sun#13: drupal.locale-import.13.patch queued for re-testing.
Comment #17
sun#13: drupal.locale-import.13.patch queued for re-testing.
Comment #18
andypostsubscribe
Comment #19
sun#13: drupal.locale-import.13.patch queued for re-testing.
Comment #20
andypostSuppose we
This should be removed because 'install' is related to module and 'enable' for themes.
All other seems good to go.
We have enough tests for module\theme enable but not sure about translation.
Powered by Dreditor.
Comment #21
sunYou describe the current state. The @todo is intentional and highlights that the theme system is missing an 'install' operation. String translations are not supposed to be re-imported when a theme is re-enabled. That's utterly wrong, and thus, I'd like to keep this comment.
Created #1067408: Themes do not have an installation status for D8.
Comment #22
andypostActually, automatic strings re-import is not a good thing but we have no install for themes :(
Comment #23
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #24
David_Rothstein CreditAttribution: David_Rothstein commentedI don't think this patch was committed correctly. Maybe the theme.inc part of it was missing?
Comment #25
bfroehle CreditAttribution: bfroehle commentedYes, the theme.inc part didn't get committed, because the patch in #13 wouldn't even apply:
I've re-rolled the patch to just that file and attached it.
Comment #26
plachAren't we missing some test coverage, then?
Comment #27
sun@plach: Don't think so. Without the follow-up in #25, HEAD merely attempts to import string translations twice.
Comment #28
andypostFor themes only.
This small WTF should be fixed ASAP
Comment #29
sun#25: 545518-follow-up-fix.patch queued for re-testing.
Comment #30
andypostComment #31
Dries CreditAttribution: Dries commentedGood catch. Committed to 8.x. Moving back to 7.x.
Comment #32
David_Rothstein CreditAttribution: David_Rothstein commentedDoesn't look like this was committed to Drupal 8 - the patch still applies.
Comment #33
sun#25: 545518-follow-up-fix.patch queued for re-testing.
Comment #35
sunWhat's happening here? D8 contains the change already.
Comment #36
sun#25: 545518-follow-up-fix.patch queued for re-testing.
Comment #37
bfroehle CreditAttribution: bfroehle commentedYou're right, the commit in 8.x was at http://drupalcode.org/project/drupal.git/commitdiff/fc5f56e9c
Comment #38
David_Rothstein CreditAttribution: David_Rothstein commentedStrange, I'm positive it wasn't there before. Maybe it was committed on April 4 but not actually pushed until later?
Anyway, #25 is certainly RTBC for Drupal 7 also, as long as the tests still pass.
Comment #39
Dries CreditAttribution: Dries commentedCommitted to 7.x.