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

Incompatible with mkdocs-material privacy plugin #196

Closed
jonaharagon opened this issue Feb 27, 2023 · 10 comments
Closed

Incompatible with mkdocs-material privacy plugin #196

jonaharagon opened this issue Feb 27, 2023 · 10 comments

Comments

@jonaharagon
Copy link
Contributor

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
Copy link
Contributor

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:

def on_post_page(self, output, page, config):
"""
Some plugins we control ourselves need this event.
"""
# Manually trigger with-pdf on_nav, see #110
with_pdf_plugin = config["plugins"].get("with-pdf")
if with_pdf_plugin:
with_pdf_plugin.on_post_page(output, page, config)
return output

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
Copy link

dngray commented Feb 28, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor

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:
squidfunk/mkdocs-material#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:
squidfunk/mkdocs-material#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
Copy link

dngray commented Feb 28, 2023

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 privacyguides/privacyguides.org#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/

🤣, that's what happens when you're tired.

@jonaharagon
Copy link
Contributor Author

jonaharagon commented Feb 28, 2023

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
Copy link
Sponsor

squidfunk commented Feb 28, 2023

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:

This class extends MkDocs' Files class to support links and assets that
have a translated locale suffix.
Since MkDocs relies on the file.src_path of pages and assets we have to
derive the file.src_path and check for a possible .<locale>.<suffix> file
to use instead of the link / asset referenced in the markdown source.

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
Copy link
Contributor

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.

def on_post_build(self, config):

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:

This is because the MkDocs `search` plugin is hardcoded in the themes
javascript sources so there can only be one search index for the whole
build.

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
Copy link
Sponsor

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
Copy link
Contributor Author

I'm going to close this issue because I feel like this might have been related to squidfunk/mkdocs-material#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.

@jonaharagon jonaharagon closed this as not planned Won't fix, can't repro, duplicate, stale Mar 22, 2023
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

No branches or pull requests

4 participants