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

Insiders privacy plugin failing on build (with malformed Google fonts stylesheets links?) #3642

Closed
5 tasks done
villeodell opened this issue Feb 28, 2022 · 9 comments
Closed
5 tasks done
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open

Comments

@villeodell
Copy link

Contribution guidelines

I've found a bug and checked that ...

  • ... the problem doesn't occur with the mkdocs or readthedocs themes
  • ... the problem persists when all overrides are removed, i.e. custom_dir, extra_javascript and extra_css
  • ... the documentation does not mention anything about my problem
  • ... there are no open or closed issues that are related to my problem

Description

I'm hoping to use the very useful privacy feature of insiders, but the mkdocs build fails when it's enabled and a custom Google font is specified.

Expected behaviour

External script plus stylesheets are successfully bundled by the privacy plugin during build without error as described at https://squidfunk.github.io/mkdocs-material/setup/ensuring-data-privacy/?h=privacy#built-in-privacy-plugin

Actual behaviour

With

privacy:
      externals: bundle
      externals_directory: assets/externals

Result is:

ERROR    -  Error building page 'index.md': 'text/html'
File "/mkdocs-material-insiders/material/plugins/privacy/plugin.py", line 111, in on_post_page
    value.replace(raw, self.__fetch(url, page)),
  File "/mkdocs-material-insiders/material/plugins/privacy/plugin.py", line 193, in __fetch
    extension = extensions[name]
KeyError: 'text/html'

With

privacy:
      externals: report
      externals_directory: assets/externals

It seems like stylesheet links

Steps to reproduce

Specify

theme:
  font:
    text: Roboto

with

plugins:
  - privacy:
      externals: bundle
      externals_directory: assets/externals

If I leave out the custom Google font, the site is built fine.

If I set

plugins:
  - privacy:
      externals: report
      externals_directory: assets/externals

The external asset for Roboto is reported as:

WARNING  -  External asset in 'index.html':
            https://fonts.googleapis.com/css?family=Roboto:300,300i,400,400i,700,700i%7C:400,400i,700,700i&display=fallback

Following the link results in a 404 error, which may be why the privacy plugin fails?

Package versions

  • Python: Python 3.9.10
  • MkDocs: mkdocs, version 1.2.3 from /usr/local/lib/python3.9/site-packages/mkdocs (Python 3.9)
  • Material: 8.2.3+insiders.4.10.0

Configuration

site_name: Test
plugins:
  - search
  - macros
  - minify:
      minify_html: true
  - privacy:
      externals: bundle
      externals_directory: assets/externals
theme:
  name: material
  font:
    text: Roboto

System information

  • Operating system: ...
  • Browser: ...
@squidfunk squidfunk added the bug Issue reports a bug label Feb 28, 2022
@FISHMANPET
Copy link
Contributor

I'm building a site that's loading fonts from Google rather than saving them locally. When the site is viewed, there's a call to the google fonts site of the same form that returns a 400 error. In my case I'm using Open Sans and this is the URL that the site tries to query:
https://fonts.googleapis.com/css?family=Open+Sans:300,300i,400,400i,700,700i%7C:400,400i,700,700i&display=fallback
Another site that was built in the past calls this working URL:
https://fonts.googleapis.com/css?family=Open+Sans:300,400,400i,700%7C&display=fallback

If I replace 700i%7C: in the 400-ing URL with 700i, the URL works. Looks like there's an issue with how that URL is generated. In addition, it's asking for the same sizes multiple times - 400, 400i, 700, 700i. %7C appears to be a | character, looks like that got erroneously inserted into the URL string.

@FISHMANPET
Copy link
Contributor

And I think it might be related to this particular change: 4f641cc
In my mkdocs.yml (and it looks like in @villeodell's as well) font.text is defined, but not font.code. If I define a font.code it goes away.
So the &7C is maybe a red herring, though it still looks like a mistake, just not one that's fatal. But if font.text is defined but not font.code, that generates a malformed URL.

@villeodell
Copy link
Author

So the &7C is maybe a red herring, though it still looks like a mistake, just not one that's fatal. But if font.text is defined but not font.code, that generates a malformed URL.

I can confirm that specifying both font.text + font.code resolves the issue; specifying just one (either text or code) results in a malformed url.

@squidfunk
Copy link
Owner

In my mkdocs.yml (and it looks like in @villeodell's as well) font.text is defined, but not font.code. If I define a font.code it goes away.

Yes, it's definitely related to font.code not being explicitly defined. The problem is that if you override font.text, MkDocs will refuse to merge it with the default value of font.code, albeit it's defined in mkdocs_theme.yml. Nonetheless, I think this is something we need to fix, and the text/html is the result of the 400 being returned by Google.

The privacy plugin is still experimental, so stuff like this is more or less expected to happen, but I think we'll get it to a stable state soon. Thanks for test-driving it, guys!

@FISHMANPET
Copy link
Contributor

To be clear, I'm not using the insiders build or the privacy plugin, this is just using the "public" theme. So in villeodell's case the bug happened to reveal itself when using the privacy plugin, but I encountered the same thing using the publicly available build. So it's bigger than just the insiders build.

@FISHMANPET
Copy link
Contributor

I'd love to throw together a PR for this because I've got an idea how to fix it, I'll probably get around to getting the dev environment all setup and going later today, but of course I won't be offended if you got to it first yourself.

@squidfunk
Copy link
Owner

I'm fairly certain it should be fixed by 64e5ed0. The defaults are now set within the template. This should also fix the issue with the privacy plugin originally reported.

@squidfunk squidfunk added the resolved Issue is resolved, yet unreleased if open label Mar 1, 2022
@FISHMANPET
Copy link
Contributor

Well that's better than what I was trying to build out...

From playing around last night I was able to trigger generation of the malformed URL, but with your code those same steps now generate a correct URL so I'd concur that this should fix the issue.

@squidfunk
Copy link
Owner

Released as part of 8.2.4 and 8.2.4+insiders-4.10.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open
Projects
None yet
Development

No branches or pull requests

3 participants