Skip to content
This repository was archived by the owner on May 15, 2019. It is now read-only.

Commit 539b056

Browse files
committed
Revert "react-components: add onResourceLoaded prop to TeX"
This reverts commit 0de8388. Looks like some mobile devices support the new font loading API, but crash on some of the particulars of how we access the font objects. Let's roll this back for now, and debug in detail later. This change fixed a non-mission-critical visual bug, so rolling it back is super safe. We'll leave the corresponding Perseus changes intact, so Perseus will continue to pass an unused `onResouceLoaded` prop to `TeX`, and that's okay. Discussion and diagnosis: https://khanacademy.slack.com/archives/mobile-1s-and-0s/p1485982023001538 The error itself: https://sentry.io/khanacademyorg/mobile-webview/issues/211287170/ Test Plan: Run tests to make sure we're not crazy, and cross fingers. Auditors: charlie, amy
1 parent d200fc4 commit 539b056

File tree

2 files changed

+0
-96
lines changed

2 files changed

+0
-96
lines changed

js/font-observer.js

Lines changed: 0 additions & 71 deletions
This file was deleted.

js/tex.jsx

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88
const PureRenderMixin = require('react-addons-pure-render-mixin');
99
const React = require('react');
1010
const ReactDOM = require('react-dom');
11-
const _ = require('underscore');
1211

13-
const FontObserver = require('./font-observer.js');
1412
const katexA11y = require('./katex-a11y.js');
1513

1614
let pendingScripts = [];
@@ -37,12 +35,6 @@ const loadMathJax = (callback) => {
3735
}
3836
};
3937

40-
// Create a FontObserver that listens for KaTeX fonts to load, all of whose
41-
// font family names start with "KaTeX".
42-
// Some browsers will quote the font family name and others won't, so we
43-
// include an optional leading quote in the regular expression.
44-
const fontObserver = new FontObserver((font) => font.family.match(/^"?KaTeX/));
45-
4638
const doProcess = () => {
4739
loadMathJax(() => {
4840
MathJax.Hub.Queue(function() {
@@ -89,14 +81,6 @@ const TeX = React.createClass({
8981
children: React.PropTypes.node,
9082
onClick: React.PropTypes.func,
9183
onRender: React.PropTypes.func,
92-
// Some resources in some browsers will, when loaded, trigger a call to
93-
// the onResourceLoaded callback. This is a hint that the appearance of
94-
// the TeX node may have changed, including its size.
95-
//
96-
// DO NOT DEPEND ON THESE EVENTS. Browser support for detecting font
97-
// loading is poor. But it's still a helpful way to resolve minor race
98-
// conditions :)
99-
onResourceLoaded: React.PropTypes.func,
10084
style: React.PropTypes.any,
10185
},
10286

@@ -106,16 +90,13 @@ const TeX = React.createClass({
10690
return {
10791
// Called after math is rendered or re-rendered
10892
onRender: function() {},
109-
onResourceLoaded: function() {},
11093
onClick: null,
11194
};
11295
},
11396

11497
componentDidMount: function() {
11598
this._root = ReactDOM.findDOMNode(this);
11699

117-
fontObserver.addListener(this.handleResourceLoaded);
118-
119100
if (this.refs.katex.childElementCount > 0) {
120101
// If we already rendered katex in the render function, we don't
121102
// need to render anything here.
@@ -169,8 +150,6 @@ const TeX = React.createClass({
169150
},
170151

171152
componentWillUnmount: function() {
172-
fontObserver.removeListener(this.handleResourceLoaded);
173-
174153
if (this.script) {
175154
loadMathJax(() => {
176155
const jax = MathJax.Hub.getJaxFor(this.script);
@@ -181,10 +160,6 @@ const TeX = React.createClass({
181160
}
182161
},
183162

184-
handleResourceLoaded: function() {
185-
this.props.onResourceLoaded();
186-
},
187-
188163
setScriptText: function(text) {
189164
if (!this.script) {
190165
this.script = document.createElement("script");

0 commit comments

Comments
 (0)