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

fix svgTextUtils for FF: use child element instead of SVG to getSize() #3783

Merged
merged 2 commits into from
Apr 17, 2019

Conversation

antoinerg
Copy link
Contributor

Closes #2616

The fix was tested in Mozilla Firefox 62.0.3 in Linux. I try to fix the following misbehaving routine in FF:

function getSize(_selection, _dimension) {
return _selection.node().getBoundingClientRect()[_dimension];
}

However, after inspecting the DOM in Firefox, it turns out that it sets the size of the embedded svg.gtitle-math elements to a very large value. However, it sizes properly its first and only child element. We use this one instead when calling getSize().

Codepen before: https://codepen.io/antoinerg/pen/oOEaPz
Codepen after: https://codepen.io/antoinerg/pen/ZZrqmd

Before:
newplot(1)
After:
newplot(2)

@etpinard
Copy link
Contributor

Brilliant fix!

Wanna try to 🔒 this down in a test?

Good news: npm run test-jasmine -- --bundleTest=mathjax --FF appears to fail on master. (1) Wanna make sure it passed off your branch? Once (1) is done, simply adding one more task (corresponding to npm run test-jasmine -- --bundleTest=mathjax --FF in test_bundle.js should be good enough.

@antoinerg
Copy link
Contributor Author

Good news: npm run test-jasmine -- --bundleTest=mathjax --FF appears to fail on master. (1) Wanna make sure it passed off your branch?

It works on branch fix-2616!

Once (1) is done, simply adding one more task (corresponding to npm run test-jasmine -- --bundleTest=mathjax --FF in test_bundle.js should be good enough.

I added a test in test_bundle.js for MathJax in Firefox in b1177a6. It gets executed on CircleCI as the last step:
Screenshot_2019-04-17_15-13-06

@etpinard etpinard added status: reviewable bug something broken labels Apr 17, 2019
@etpinard
Copy link
Contributor

One of my favourite bug fixes of the year so far.

💃 💃 💃

@jonmmease
Copy link
Contributor

Awesome!!! 🎉
This one has annoyed me ever since I got things working again in chrome last year. Thanks for digging in!

@etpinard
Copy link
Contributor

To be released in tomorrow morning's v1.47.3

@antoinerg antoinerg merged commit 424c686 into master Apr 17, 2019
@antoinerg antoinerg deleted the fix-2616 branch April 17, 2019 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mathjax doesn't render for firefox
3 participants