ultrabug / mkdocs-static-i18n

MkDocs i18n plugin using static translation markdown files
https://ultrabug.github.io/mkdocs-static-i18n/
MIT License
228 stars 38 forks source link

Incompatible with mkdocs-material privacy plugin #196

Closed jonaharagon closed 1 year ago

jonaharagon commented 1 year ago

https://squidfunk.github.io/mkdocs-material/setup/ensuring-data-privacy/#built-in-privacy-plugin

The privacy plugin is supposed to automatically download mermaid.min.js and replace https://unpkg.com/mermaid@9.3.0/dist/mermaid.min.js in bundle.ce72ebac.min.js with https://<my domain>/assets/external/unpkg.com/mermaid@9.3.0/dist/mermaid.min.js.

This works fine with a standard mkdocs-material install, but after enabling the i18n plugin this no longer works. The mermaid.min.js file gets downloaded, but the URL in the theme's JS file no longer gets updated by the privacy plugin, so it still makes a connection to the external CDN.


I can recreate this issue with the following minimal config:

mkdocs.yml:

site_name: My Docs
theme:
  name: material

markdown_extensions:
  - pymdownx.superfences:
      custom_fences:
        - name: mermaid
          class: mermaid
          format: !!python/name:pymdownx.superfences.fence_code_format

plugins:
  - privacy
  - i18n:
      default_language: en
      languages:
        en:
          name: English

docs/index.en.md:

# Mermaid Diagram

``` mermaid
graph TB
    Start[Start] --> anonymous{Trying to be<br> anonymous?}
    anonymous--> | Yes | tor(Use Tor)
    anonymous --> | No | censorship{Avoiding<br> censorship?}
    censorship --> | Yes | vpnOrTor(Use<br> VPN or Tor)
    censorship --> | No | privacy{Want privacy<br> from ISP?}
    privacy --> | Yes | vpnOrTor
    privacy --> | No | obnoxious{ISP makes<br> obnoxious<br> redirects?}
    obnoxious --> | Yes | encryptedDNS(Use<br> encrypted DNS<br> with 3rd party)
    obnoxious --> | No | ispDNS{Does ISP support<br> encrypted DNS?}
    ispDNS --> | Yes | useISP(Use<br> encrypted DNS<br> with ISP)
    ispDNS --> | No | nothing(Do nothing)
kamilkrzyskow commented 1 year ago

Does it happen for all languages, even the default one? I don't have access to Insiders nor the Privacy plugin to test, but the i18n plugin triggers external event functions manually in some cases like here: https://github.com/ultrabug/mkdocs-static-i18n/blob/2d1f3c6330e92bbe9ce3c67f1e9e9657a0b19091/mkdocs_static_i18n/plugin.py#L539-L547 I would assume that the issue here could be quite similar to #110 maybe changing the order of the plugins in the mkdocs.yml file or changing the priority of the events could help.

dngray commented 1 year ago

Does it happen for all languages, even the default one?

Yes, it happens with the default language.

maybe changing the order of the plugins in the mkdocs.yml

I guess I could get started on that to see if it fixes it.

jonaharagon commented 1 year ago

All languages including the default, even if the default language is the only language when this plugin is enabled. Moving the privacy plugin before or after i18n in mkdocs.yml doesn't make a difference unfortunately.

kamilkrzyskow commented 1 year ago

Then unfortunately I'm out of ideas here. As for my additional 2 cents of unsolicited advice, I think that https://www.privacyguides.org/ is "too big" (if that's the right term) to be using this plugin in its current state. Your search_index.json is already quite big (2.8 MB), as currently all languages are joined into one file. You're also using a lot of Insiders features of the Material theme, which could happen to not be supported by this plugin.

I think you might be better off with using the MkDocs recommended way and create separate language builds and join them together: https://github.com/squidfunk/mkdocs-material/discussions/2346 Live example of this approach (not exactly for the guide above): https://github.com/tiangolo/fastapi

The biggest issue people seem to have apart of the build process is the language switcher, but that also seems to be in development: https://github.com/squidfunk/mkdocs-material/issues/4835 If you have the same names of files for each language, then you might also add some JavaScript extra file which modifies the links in the switcher after the site loads.

Of course this plugin has it's perks like the custom sitemap.xml etc., just wanted to speak up with my concerns. If you're still strong about using this plugin and aren't against JavaScript additions then maybe you'd be interested in a script that assures the same site language in the search results. You could adapt it from here. I admit it's a little bit "opinionated", but I hope it's a good reference point.

I'm also unsure how well your site search works without the fixes I've added in #182 🤔 But you still got time before you make your final choice, as you have 1 month until you release your i18n site 😉
https://blog.privacyguides.org/2023/03/26/i18n-announcement/

dngray commented 1 year ago

I think that https://www.privacyguides.org/ is "too big" (if that's the right term) to be using this plugin in its current state. Your search_index.json is already quite big (2.8 MB), as currently all languages are joined into one file.

We did notice this https://github.com/privacyguides/privacyguides.org/issues/1930 before we added the other languages, likely this hasn't helped though.

But you still got time before you make your final choice, as you have 1 month until you release your i18n site wink https://blog.privacyguides.org/2023/03/26/i18n-announcement/

:rofl:, that's what happens when you're tired.

jonaharagon commented 1 year ago

I think you might be better off with using the MkDocs recommended way and create separate language builds and join them together:

This is what I was thinking recently, but aside from the couple issues I opened here this plugin has worked well, I like the automatic hreflang attributes in the head and the fact that the language switcher keeps you on the page you're on, which I don't think is possible with the regular configuration. I'll probably still look into that more anyways.

squidfunk commented 1 year ago

Just to chim in: The privacy plugin makes use of event priorities, so ordering should not matter for most plugins, as long as they don't define priorities themselves. Additionally, the privacy plugin got a rewrite, it's now concurrent and adds the downloaded files to MkDocs own files collection, making them visible to other plugins. I could imagine that this might be a problem with the current implementation of this plugin. I'm not familiar with the implementation, but when I read the source code, it seems that this plugin partially circumvents MkDocs event system. This might be problematic.

Also, there seems to be a separate file implementation, so maybe .min is picked up as a locale/language:

https://github.com/ultrabug/mkdocs-static-i18n/blob/2d1f3c6330e92bbe9ce3c67f1e9e9657a0b19091/mkdocs_static_i18n/suffix_structure.py#L18-L23

Additionally, as already mentioned, (auto-)linking pages between languages will soon come to Material for MkDocs. For this to work properly, the sitemaps must be separate for each language, as they are heavily used by the theme. I'm, not sure how this plugin implements it, but if you have separate MkDocs builds, this is guaranteed.

kamilkrzyskow commented 1 year ago

The plugin doesn't make separate distinct MkDocs builds, after it reaches the on_post_build event it manually calls functions to mimic the act of building pages. There is 1 single sitemap with all the pages, and there is 1 single search_index.json that contains all of the searchable content. Basically the plugin tries to allow complete i18n localization within 1 mkdocs.yml file, which counters the basic MkDocs idea of 1 language = 1 mkdocs.yml. This leads to a lot of issues, because most other plugins abide by the rules and stick to the same idea of 1 config = 1 language, and then this plugin needs to call other plugins functions during subsequent mimic builds swapping their configuration on the fly.

https://github.com/ultrabug/mkdocs-static-i18n/blob/2d1f3c6330e92bbe9ce3c67f1e9e9657a0b19091/mkdocs_static_i18n/plugin.py#L549 And the plugin does already provide auto-linking of pages in the language switcher, because it enforces a static file naming scheme (aka name.lang.md) where all localized versions share the URI suffix, and differ in the language prefix, so let's hope that your additions won't cause breaking changes here 🤞

There is a line in the readme.md https://github.com/ultrabug/mkdocs-static-i18n/blob/main/README.md#compatibility-with-the-search-plugin: https://github.com/ultrabug/mkdocs-static-i18n/blob/2d1f3c6330e92bbe9ce3c67f1e9e9657a0b19091/README.md?plain=1#L396-L398 As you @squidfunk have created a rewrite of the search plugin do you think it would be possible to add some kind of theme.config.search_index_path or plugin.config.sitemap_path option to allow this i18n plugin to influence the JavaScript hard-coded path in your theme? https://github.com/squidfunk/mkdocs-material/blob/8014c0423c4844d94ae3fd04095f06a2e22e352d/src/assets/javascripts/bundle.ts#L114 https://github.com/squidfunk/mkdocs-material/blob/8014c0423c4844d94ae3fd04095f06a2e22e352d/src/assets/javascripts/integrations/sitemap/index.ts#L97

squidfunk commented 1 year ago

As you @squidfunk have created a rewrite of the search plugin do you think it would be possible to add some kind of theme.config.search_index_path or plugin.config.sitemap_path option to allow this i18n plugin to influence the JavaScript hard-coded path in your theme?

That doesn't sound like a good idea to me. There are several plugins other than the built-in search plugin that rely on the file being called search_index.json, so all of those plugins would need to be adjusted and then brought similarily under governance of this plugin. That sounds very fragile to me, so nothing I would commit to support. From the top of my head:

Same goes for sitemap.xml.

jonaharagon commented 1 year ago

I'm going to close this issue because I feel like this might have been related to https://github.com/squidfunk/mkdocs-material/issues/5127 (which is fixed), but I no longer use this plugin to test whether this is actually fixed, so I won't clutter up your issue tracker if nobody else uses the insiders privacy plugin alongside this one.