Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#2263 Do not try and store items in the session if the sessio… #19245

Merged
merged 1 commit into from
Dec 23, 2020

Conversation

seamuslee001
Copy link
Contributor

…n is currently empty

Store the locale in the session even if we are in a single lingual instance

Overview

This aims to fix an issue where by if you have multiple languages enabled on your civicrm instance the KcFinder breaks because the session is tried to written to before Drupal has booted

Before

  1. Create a Drupal test site
  2. Enable multiple languages
  3. Try to access KCFinder say from managing a contribution page and get errors

After

The above works

Technical Details

There is more investigation information found on the alternate PR #19242. However this to me tries to target the actual underlying source of the problem where was that feels like it is just putting a bandaid on the issue

ping @haystack @kainuk @mlutfy

Are any of you able to test this and let me know if it works for you?

…n is currently empty

Store the locale in the session even if we are in a single lingual instance
@civibot
Copy link

civibot bot commented Dec 21, 2020

(Standard links)

@kainuk
Copy link
Contributor

kainuk commented Dec 21, 2020

Hi @seamuslee001, I just tested this on my development database and on the site of the customer. It works fine, thanks. I will close my alternative PR

@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label Dec 21, 2020
@eileenmcnaughton
Copy link
Contributor

merging based on @kainuk's testing & looking at the code this looks like it should be merged. I've added 'merge-ready' to give others a couple of days to weigh in

@mlutfy
Copy link
Member

mlutfy commented Dec 21, 2020

Looks good!

@colemanw colemanw merged commit 3e76fbf into civicrm:5.33 Dec 23, 2020
@eileenmcnaughton eileenmcnaughton deleted the dev_core_2263 branch December 23, 2020 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.33 merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants