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

Make polyfill optional, at least. #10354

Closed
Axel-Naumann opened this issue Oct 12, 2023 · 13 comments
Closed

Make polyfill optional, at least. #10354

Axel-Naumann opened this issue Oct 12, 2023 · 13 comments
Labels
enhancement a request to enhance doxygen, not a bug HTML HTML / XHTML output

Comments

@Axel-Naumann
Copy link

Describe the bug
Requesting mathJax unconditionally pulls in polyfill.io. GDPR-wise that's a problem, at least for us. Also, Browser Support for ES6 (2015) suggests that this should really not be an issue anymore.

If really need be this could become configurable.

Expected behavior
I can use doxygen-generated documentation (including MathJax) without connecting other servers.

To Reproduce
See e.g. the network trace of https://root.cern/doc/v628/

Version
That's probably not really relevant here - but we use doxygen from Ubuntu 22.04.

Additional context
On this topic, MathJax says

In particular, to allow MathJax version 3 to work with IE11,

IE11 is dead, please :-)

@Axel-Naumann
Copy link
Author

I'm happy to provide a PR, ideally just removing the relevant line. Lmk whether that's desirable.

@albert-github albert-github added enhancement a request to enhance doxygen, not a bug HTML HTML / XHTML output labels Oct 12, 2023
@albert-github
Copy link
Collaborator

IE11 might look like to be dead but

  • the mentioned notice suggests to me that it is still possible to use IE11 to 2029
  • there will be still quite a few systems around that use IE11 (locally and probably also with a local version of MathJax)

Seen this I don't think it is possible yet to remove this line completely, a possibility might be to have (in the resulting HTML file a construct that detects whether or not IE11 is used and when that is the case the part

"<script src=\"https://polyfill.io/v3/polyfill.min.js?features=es6\"></script>\n"

is enabled.

@Axel-Naumann
Copy link
Author

Thanks a lot, @albert-github ! The problem I'm trying to solve is that we want our Doxygen doc (which uses MathJax) to be GDPR compliant. For that we must not pull in polyfill, not even if someone uses IE11.

Your comment suggests that this would need to become a configuration option? Maybe as MATHJAX_VERSION = MathJax_3_nopoly to not blow up the number of parameters? Or

# Dynamically load PolyFill's ES6 support into generated webpages for MathJax v3.
# Requires MATHJAX_VERSION set to MathJax_3.
MATHJAX_POLYFILL = YES

All of this is obviously really your call, I'm just suggesting possible approaches hoping that you would agree that GDPR compliance is good to offer, and that you'd tell me how I can contribute to make it happen!

@albert-github
Copy link
Collaborator

I don't think the polyfill can be removed for IE11 seen the comment you cited from MathJax.
No I'm not suggesting to add an new option for the setting MATHJAX_VERSION or a completely new setting MATHJAX_VERSION (in case it would be necessary the extension of MATHJAV_VERSION would be preferred though).
I think it should be possible in the HTML code to detect the used browser and only when this is IE11 the polyfill should be loaded / enabled.

@Axel-Naumann
Copy link
Author

I understood your suggestion - apologies for not making that clear. But this means that polyfill will be loaded, even if only in certain cases, and thus websites using Doxygen documentation will not be GDPR compliant.

@albert-github
Copy link
Collaborator

When I understand it well the polyfill has the problem, shouldn't the problem be reported there or with MathJax?

@Axel-Naumann
Copy link
Author

Well, GDPR basically says (INAL) that you cannot use US-based services. polyfill is US based. MathJax works well without polyfill (except IE11). So not sure what to report to either polyfill or MathJax here...

It's doxygen that unconditionally pulls in polyfill if MathJax3 is on, and that's what I need to fix.

@doxygen
Copy link
Owner

doxygen commented Oct 16, 2023

According to this site the worldwide use of IE has dropped to 0.22%. Of those users only a small subset will be using it for doxygen pages, and an even smaller subset will be using Mathjax, so I don't mind to remove the polyfill dependency, and wait for the unlikely bug report that someone finds this a problem.

@albert-github
Copy link
Collaborator

albert-github commented Oct 17, 2023

It doesn't look like much users, but it always depends on what one is measuring, when measuring social networks the results will be different from technical networks. Also there are companies that still use systems that are not (directly) connected to the internet and that run old versions of OS. The later will not likely be using the newest versions of doxygen or update to it so the risk would be small.

Maybe the suggested MATHJAX_VERSION = MathJax_3_nopoly is an easy and good alternative but in the form MATHJAX_VERSION = MathJax_3_polyfill so the "normal" user can user can use MATHJAX_VERSION = MathJax_3 and for some dedicated systems the version with with polyfill can be enabled.

@Axel-Naumann
Copy link
Author

No-network doesn't seem like a scenario where loading polyfill.io would work anyway?

@albert-github
Copy link
Collaborator

Indeed.
With the MathJax 2 version it was "easy" to create a local, when necessary stripped down, version of MathJax and use this.
With MathJax 3 I never tried a local version but seen the "polyfill" requirement a no-network version won't work at all even when having a local polyfill.js as the polyfill is not configurable.
Regarding the upcoming MathJax 4 I don't know what the possibilities for a no network / local version might be.

@doxygen
Copy link
Owner

doxygen commented Oct 21, 2023

I've removed the reference to polyfill.

If someone still needs it do the following:
Generate a custom header

doxygen -w html header.html footer.html style.css

Edit header.html and add <script src="https://polyfill.io/v3/polyfill.min.js?features=es6"></script> in front of
$mathjax
Use the custom header by setting this in the Doxyfile:

HTML_HEADER            = header.html

@doxygen doxygen added the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label Oct 21, 2023
@doxygen
Copy link
Owner

doxygen commented Dec 25, 2023

This issue was previously marked 'fixed but not released',
which means it should be fixed in doxygen version 1.10.0.
Please verify if this is indeed the case. Reopen the
issue if you think it is not fixed and please include any additional information
that you think can be relevant (preferably in the form of a self-contained example).

@doxygen doxygen removed the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label Dec 25, 2023
@doxygen doxygen closed this as completed Dec 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement a request to enhance doxygen, not a bug HTML HTML / XHTML output
Projects
None yet
Development

No branches or pull requests

3 participants