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

Get i18n_node submodule working #8

Closed
indigoxela opened this issue Feb 28, 2020 · 9 comments · Fixed by #10
Closed

Get i18n_node submodule working #8

indigoxela opened this issue Feb 28, 2020 · 9 comments · Fixed by #10

Comments

@indigoxela
Copy link
Member

indigoxela commented Feb 28, 2020

i18n_node isn't working yet.

How to reproduce:

Enable Internationalization and Multilingual content modules, go to admin/structure/types/manage/page

Besides several property: stdClass notices (different issue) there's an error:

Error: Call to undefined function variable_type_include() in i18n_node_form_node_type_form_alter() (line 419 of .../modules/i18n/i18n_node/i18n_node.module).

Function variable_type_include is provided by the variable module in Drupal. That module doesn't exist for Backdrop. Possibly it needs to get migrated, but maybe core or i18n module could provide the required functionality.

I'm aware that this module is far from being ready to release, but I thought a php error is worth an issue.

@indigoxela
Copy link
Member Author

I did some recherche.

In backdrop-ops/contrib#34 the module variable has been marked as obsolete (CMI in core), but that's only half of the truth, I guess.
In D8 variable has been dropped, leaving a gap for translatable variables.

I18n isn't the only module relying on variable, if looking at the install counts for Drupal:

  • i18n: 159,731 sites report using this module
  • variable: 201,051 sites report using this module

Looking at the above numbers, I guess we should port variable for compability reasons.

My question is: has this been discussed before?

@docwilmot
Copy link
Member

My question is: has this been discussed before?

Dont think so. I get the impression that Variable Module isnt a necessary module even for sites using it. I think I usually just get rid of any code mentioning that module, and never missed it.

@indigoxela
Copy link
Member Author

indigoxela commented Mar 4, 2020

I usually just get rid of any code mentioning that module, and never missed it.

Sounds reasonable. Maybe the modules' install count is misleading. Mainly D7 i18n and domain module are depending on it and that could be done better/easier now with CMI, at least in i18n_node. I'll give it a try. 😉

indigoxela added a commit to indigoxela/i18n that referenced this issue Mar 4, 2020
@indigoxela
Copy link
Member Author

indigoxela commented Mar 4, 2020

So here's a PR.

It's mostly about i18n_node submodule ("Multilingual content")
The module installs and seems to work (mostly), but the PR sure needs more work.

All sorts of testing is welcome!

@indigoxela indigoxela changed the title Call to undefined function variable_type_include() Get i18n_node submodule working Mar 5, 2020
@indigoxela
Copy link
Member Author

indigoxela commented Mar 5, 2020

I believe, the Multilingual content submodule is fully working now. (Note: Simpletests aren't implemented yet)

How to test:

  • Enable only this submodule with its dependencies (some other submodules don't work yet)
  • Enable translations for a node type
  • Play with the node options on admin/config/regional/i18n/node
  • Play with the i18n node settings on admin/structure/types/manage/YOURTYPE
  • Inspect dblog for any errors

... and report any problems here.

@laryn
Copy link
Member

laryn commented Mar 5, 2020

My feedback:

  • I had metatag installed on the dev site I was testing on and got a fatal error when enabling i18n.
  • I had to clear caches to see admin/config/regional/i18n/node
  • Clicking around a bit I'm starting to see a lot of these: Notice: Undefined property: stdClass::$language in i18n_node_tokens() (line 34 of /path/modules/i18n/i18n_node/i18n_node.tokens.inc).

Other than that things look good so far!


Changing line 34 to this seems to work:

$langcode = isset($options['langcode']) ? $options['langcode']->langcode : i18n_langcode();

@indigoxela
Copy link
Member Author

@laryn many thanks for testing!

Re metatag and fatal:

Did that happen when enabling the main i18n module or the i18n_node submodule?

Re clear caches:

Strange... I'll try to reproduce - it didn't happen to me.

Re Notice: Undefined property: stdClass::$language:

Many thanks for finding that missing language-langcode shuffle.
I hope, now we catched them all. 😉

But...
When debugging i18n_node_tokens(), the $options array looked like this:

array (
  'sanitize' => false,
  'clear' => true,
  'callback' => 'path_clean_token_values',
  'langcode' => 'en',
)

It's a string, not an object, at least when saving a node, I've updated my PR accordingly. Or did you see an object at some point?

@indigoxela
Copy link
Member Author

OK, regarding metatag:

When having metatag module enabled and enabling i18n_string submodule, I get:

Error: Call to undefined function ctools_include() in metatag_i18n_list_metatag_config() (line 134 of .../modules/metatag/metatag.i18n.inc).
So that's a bug in metatag.i18n.inc, ... ha!, you already reported it there. 👍

And I suspect, it was that fatal in module install that was causing your need to flush caches.

@indigoxela
Copy link
Member Author

Two more commits and now I'm almost sure, that i18n_node works properly.

I fixed the broken content type (name, help...) translation and the weird call of language_list ( will fix issue #9).

Someone could merge this PR, I guess. 😉 (I really try to get around becoming an i18n maintainer, as this module needs so much more work and is complex...)

docwilmot added a commit that referenced this issue Mar 6, 2020
Issue #8: Get rid of variable module dependency in i18n_node
@docwilmot docwilmot mentioned this issue Mar 7, 2020
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants