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.
Follow-up from #1971384: [META] Convert page callbacks to controllers.
Comment | File | Size | Author |
---|---|---|---|
#25 | 2086485-25.patch | 20.24 KB | pwolanin |
#19 | drupal8.language-module.2086485-19.patch | 25.32 KB | jibran |
#19 | interdiff.txt | 5.44 KB | jibran |
#16 | drupal8.language-module.2086485-16.patch | 25.04 KB | disasm |
#16 | interdiff.txt | 907 bytes | disasm |
Comments
Comment #1
brianV CreditAttribution: brianV commentedI'll take a crack at this in the morning!
Comment #2
brianV CreditAttribution: brianV commentedFirst pass
Comment #3
brianV CreditAttribution: brianV commentedWith whitespace fixes dreditor hinted at.
Comment #4
brianV CreditAttribution: brianV commentedSorry, one final niggling whitespace fix.
Comment #6
brianV CreditAttribution: brianV commentedThis patch backs off the Local Tasks changeover as that's still in flux and was causing test failures.
The patch has two failures left that I can't figure out. Apparently, somehow, my changes to the language negotiation form controller cause the site to redirect incorrectly after the a different default language is selected on the regional settings page. It's not clear how changes to this route and controller are affecting that one.
Open to any ideas about these failures.
Comment #8
jibranComment #9
brianV CreditAttribution: brianV commentedjibran - thanks for updating the status.
It was inexplicably failing tests; now it works! Must have been a bad test or something that got fixed...
Comment #10
dawehner@inheritdoc
Isn't there $this->config() available?
>80 chars.
Is there a reason to not inject it?
All of the t() should be $this->t()
Comment #11
jibranNW as per #10.
Comment #12
jibranSome fixes.
Comment #14
jibran:/
Comment #16
disasm CreditAttribution: disasm commentedslight typo, this should fix it.
Comment #17
disasm CreditAttribution: disasm commentedComment #18
disasm CreditAttribution: disasm commentedI'd prefer to store the entire configFactory so if another module extends this, and needs a different config, they'd be able to get it still, without reinjecting the configFactory. If you aren't going to store the factory, at least specify what config this is instead of just $config.
Comment #19
jibranSome fixes and tested manually works fine.
Comment #20
disasm CreditAttribution: disasm commentedI can live with that. If it passes, RTBC.
Comment #21
disasm CreditAttribution: disasm commentedoops
Comment #22
disasm CreditAttribution: disasm commentedjust can't win...
Comment #24
Hydra CreditAttribution: Hydra commentedSeems like the patch no longer applies
Comment #25
pwolanin CreditAttribution: pwolanin commentedHere's a re-roll and minor tweaks around the handling of the block manager.
Comment #26
dawehnerI manually tested the page and it worked as expected. The code looks fine as well.
Comment #27
webchickCommitted and pushed to 8.x. Thanks!