page.changeSlug:before ->languageCode parameter always return null

[bvdputte]

Description

Following https://getkirby.com/docs/reference/plugins/hooks/page-changeslug-before, languageCode should return the language code as string, but it's always null.

Expected behavior

languageCode returns the correct languageCode.

To reproduce

Setup a multilanguage website in Kirby, add this hook, and check for the languageCode parameter.

'page.changeSlug:before' => function ($page, $slug, $languageCode) {
    ray($page);
    ray($slug);
    ray($languageCode);
},

Your setup

Kirby Version
3.8.3

Your system (please complete the following information)

• Device: Mac
• OS: macOS 12.6.1

Additional context

After a little bit of digging I found out languageCode is never set here: https://github.com/getkirby/kirby/blob/7dcf359e3da2fa09923a3e0d851f2a74ae668e78/src/Cms/PageActions.php#L214

Changing $languageCode to $language->code() here: https://github.com/getkirby/kirby/blob/7dcf359e3da2fa09923a3e0d851f2a74ae668e78/src/Cms/PageActions.php#L226 seems to fix it. But I don't really "understand" the flow, and assume it should come in as a parameter somehow (which it doesn't, it's always null).

[distantnative]

@bvdputte is it really never set or just always null for the default language, but the correct language code when changing the slug of a non-default language?

[lukasbestle]

I think it's never set. I can't see any place in the core that calls changeSlug() and therefore changeSlugForLanguage() with its second argument at all.

[distantnative]

The suggested change makes sense anyways IMO as whatever logic happens in App::language() should be taken advantage of.

[bvdputte]

@bvdputte is it really never set or just always null for the default language, but the correct language code when changing the slug of a non-default language?

In my tests it's never set.

[bvdputte]

Maybe also remove that parameter from the changeSlugForLanguage method when it's not used at all?

[distantnative]

It will be used when a developer calls $page->changeSlug() with the language code. So that would be a breaking change (and not a good one) if we would remove it. Only ourselves we call the methods without the parameter and rely on the automatic detection of the current language via App::language().

[distantnative]

✅