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

Error: Call to a member function init() on null in view->init_localization() on updating core (latest release) #4626

Closed
indigoxela opened this issue Sep 18, 2020 · 16 comments · Fixed by backdrop/backdrop#3315

Comments

@indigoxela
Copy link
Member

indigoxela commented Sep 18, 2020

Description of the bug

This already did pop up in another issue while testing the preview release, but we missed the actual root cause.

Steps To Reproduce

  1. Install an outdated Backdrop version (like 1.16.2)
  2. Enable the locale and language modules and add any language (translations aren't needed)
  3. Replace core directory with the one from 1.17.0 (or 1.16.3)
  4. Visit the start page or any other page containing a view - before flushing caches or running the updater

Actual behavior

Error: Call to a member function init() on null in view->init_localization() (line 2009 of .../core/modules/views/includes/view.inc).

Expected behavior

No error, of course.

Many people probably run the updater immediately after replacing files, but others may not and the site shouldn't look broken then - even if flushing caches fixes the problem.

People who are running core updates via UI won't get that error, neither people who run drush updb or b updb right after replacing files.

Additional information

It's too late for the 1.16.3/1.17.0 releases, but we could fix it for people who didn't update yet.

Possibly by catching it in function init_localization() - if $this->localization_plugin is still NULL, return instead of trying to act on it?

@indigoxela
Copy link
Member Author

Here's a PR that catches the problem.

To test it, follow the steps in the issue description.

@herbdool
Copy link

I think it looks good I'll test later. Wondering when it'll actually ever be "none"? Maybe that part isn't working?

@indigoxela
Copy link
Member Author

indigoxela commented Sep 23, 2020

Wondering when it'll actually ever be "none"?

That's an admin setting. Localization can be turned off on admin/structure/views/settings/advanced
Translation method can be "core" or "none". Both ship with views.

In rare cases no localization_plugin is available (file not included) - which leads to the error.
Especially in the latest update, because the file containing these plugins is new.

@klonos
Copy link
Member

klonos commented Sep 30, 2020

Code looks good, but there's punctuation missing in the inline comments, and they wrap into the next line while more words still fit. Testing on my local next...

@klonos klonos added this to the 1.17.1 milestone Sep 30, 2020
@indigoxela
Copy link
Member Author

indigoxela commented Sep 30, 2020

but there's punctuation missing in the inline comments

Where?

and they wrap into the next line while more words still fit

One full sentence per line - for readability.

(That a comment line may not be longer than 80 chars doesn't mean is has to be at least 80 chars. 😉 )

@klonos
Copy link
Member

klonos commented Sep 30, 2020

Where?

I'll add comments with suggestions in the PR ...but mostly conditional clauses.

One full sentence per line - for readability.

Nope. That's been widely used by some devs (the Views module is a huge offender of that), but it's against both Drupal and Backdrop standards. See https://api.backdropcms.org/doc-standards#general (emphasis mine):

Lines containing comments (including docblocks) must wrap as close to 80 characters as possible without going over, with a few exceptions (noted in the Tag Reference below).

@indigoxela
Copy link
Member Author

... must wrap as close to 80 characters as possible without going over ...

Means: not longer - but shorter is not forbidden.

@klonos
Copy link
Member

klonos commented Sep 30, 2020

without going over

^^ this means "not longer"

as close to 80 characters as possible

^^ this means "fit as many words as possible" (in the same line)

@indigoxela
Copy link
Member Author

I disagree but did it.

@klonos
Copy link
Member

klonos commented Sep 30, 2020

I disagree but did it.

This means that we need to update/clarify our coding standards wording, so that there is no disambiguation: #4666 ...with #3213 this becomes more important.

@indigoxela
Copy link
Member Author

Don't forget to actually test the PR... 😄

@klonos
Copy link
Member

klonos commented Sep 30, 2020

Yup, I'm doing that now... 😅

@klonos
Copy link
Member

klonos commented Sep 30, 2020

...OK, here's what I did:

  • git clone fresh
  • git checkout 1.16.0
  • install Backdrop
  • enable all errors in admin/config/development/logging
  • enable Language and Locale modules
  • add a language
  • git checkout 1.x core
  • visit home page -> error 👎

Then:

  • destroyed my local
  • git checkout -b issue-4626
  • git fetch origin pull/3315/head:issue-4626
  • git checkout 1.16.0
  • install Backdrop
  • enable all errors in admin/config/development/logging
  • enable Language and Locale modules
  • add a language
  • git checkout issue-4626 core
  • visit home page -> no error 👍

@klonos
Copy link
Member

klonos commented Sep 30, 2020

Thanks for putting up with my pedantic PR comments re code standards compliance and grammar rules re punctuation @indigoxela 🙏 ...this is now RTBC.

@indigoxela
Copy link
Member Author

indigoxela commented Oct 1, 2020

...and too late for 1.17.1 or 1.16.4 So this fix probably isn't urgent anymore. 😞

@ghost ghost modified the milestones: 1.17.1, 1.17.2 Oct 13, 2020
@ghost
Copy link

ghost commented Oct 13, 2020

Thanks @indigoxela, @klonos & @herbdool. I've merged backdrop/backdrop#3315 into 1.x and 1.17.x.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants