dasch-swiss / beol

Bernoulli-Euler OnLine
https://beol.dasch.swiss
GNU Affero General Public License v3.0
0 stars 1 forks source link

Remove the blocking proxy url #177

Closed SepidehAlassi closed 4 years ago

SepidehAlassi commented 4 years ago

resolves DSP-695

As a result of this fix, images embedded as SVG in body of Leibniz letters should be displayed. To check see leibniz:letter with id=l37205

tobiasschweizer commented 4 years ago

Ok, I will check tomorrow!

tobiasschweizer commented 4 years ago

Sometimes, I get an error message from MathJax:

Screenshot 2020-09-24 at 09 45 59

SepidehAlassi commented 4 years ago

Sometimes, I get an error message from MathJax:

Screenshot 2020-09-24 at 09 45 59

yes, I am aware of that, and Leibniz edition people are also. Their script enforces ignoring MathJax errors.

If you have a look at another letter "l37244" it has both figures and math rendered correctly. 99% of the letters render the math correctly, only 1% have problem with undefined macros, that I have talked with editors and I hope they solve. I cannot do anything about that on BEOL side because the htmls are fetched on Fly and must be represent as they are without any modification on our side.

The problem with macros in MathJax is not the concern of this PR anyway but thanks for your remark.

tobiasschweizer commented 4 years ago

Sometimes, I get an error message from MathJax: Screenshot 2020-09-24 at 09 45 59

yes, I am aware of that, and Leibniz edition people are also. Their script enforces ignoring MathJax errors.

If you have a look at another letter "l37244" it has both figures and math rendered correctly. 99% of the letters render the math correctly, only 1% have problem with undefined macros, that I have talked with editors and I hope they solve. I cannot do anything about that on BEOL side because the htmls are fetched on Fly and must be represent as they are without any modification on our side.

The problem with macros in MathJax is not the concern of this PR anyway but thanks for your remark.

I noticed that each replacement of the urls triggers the rendering. Could the replacements be done before the html is passed to the mathjax directive to be rendered?

SepidehAlassi commented 4 years ago

I noticed that each replacement of the urls triggers the rendering. Could the replacements be done before the html is passed to the mathjax directive to be rendered?

What do you mean by the replacement of URL? The only thing the code does is parsing out SVG elements from JSON and adding them directly to the text body. Then the text body is displayed, hence rendered with MathJax.

SepidehAlassi commented 4 years ago

@tobiasschweizer PS, as I mentioned, the problem with MathJax is not related to this PR and can be dealt with separately. Hopefully we would not even need to do anything if Leibniz edition people fix it on their side.

tobiasschweizer commented 4 years ago

I noticed that each replacement of the urls triggers the rendering. Could the replacements be done before the html is passed to the mathjax directive to be rendered?

What do you mean by the replacement of URL? The only thing the code does is parsing out SVG elements from JSON and adding them directly to the text body. Then the text body is displayed, hence rendered with MathJax.

private getLeibnizLetterBody(contents) {
        const html = new DOMParser().parseFromString(contents.response.docs[0].volltext, 'text/html');
        this.getLeibnizImages(html.body);
        this.letter = html.body;
    }

this.letter is used in the template and rendered by MathJax. Since getLeibnizImages performs several async calls, this.letter is updated several times, triggering MathJax to re-render the math several times. My question was if the replacement of the svgs could be finished first, and then the result would be passed to the MathJax directive, rendering it only once.

tobiasschweizer commented 4 years ago

@tobiasschweizer PS, as I mentioned, the problem with MathJax is not related to this PR and can be dealt with separately. Hopefully we would not even need to do anything if Leibniz edition people fix it on their side.

Ok, that would be great.

SepidehAlassi commented 4 years ago

Since getLeibnizImages performs several async calls, this.letter is updated several times,

@tobiasschweizer getLeibnizImages does not change the state of this.letter. If you have a closer look at the getLeibnizImages function you see that it gets the HTML body of a leibniz letter as the parameter element, then substitutes all image references in it with actual <svg> elements. and returns the updated result which is then only once assigned to this.letter. As you can then see, replacement of the images does not update the this.letter several times as you assume!

So what you are suggesting, the code is actually doing!

SepidehAlassi commented 4 years ago

@tobiasschweizer I refactored the code to make it clear that the state of this.letter changes only once.

SepidehAlassi commented 4 years ago

@flavens Thank you very much for your quick review!