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

Add MathML support #12836

Merged
merged 56 commits into from
Jan 23, 2018
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
7871260
amp-mathml based on amp-gist: first pass
Jan 12, 2018
4197b1c
update first pass
Jan 12, 2018
555d3c1
simplify callback
Jan 12, 2018
b614b65
cleanup
Jan 12, 2018
9fe5d53
no tests for now
Jan 12, 2018
ff4dd23
Rendering! First pass at resizing
Jan 13, 2018
d96cafc
cleanup
Jan 13, 2018
f1c81b6
remove mathjax-node
Jan 13, 2018
248fcd0
add mathml.js, first pass
Jan 16, 2018
0085161
make example methml items responsive
Jan 16, 2018
70160b0
register mathml integration
Jan 16, 2018
45064bf
amp-mathml cleanup
Jan 16, 2018
744208a
more cleanup
Jan 16, 2018
4a89349
copyright should be 2018
Jan 17, 2018
c2e79d1
remove some console logging
Jan 17, 2018
b204fb2
inline docs: mathml not gist
Jan 17, 2018
8afa5c1
use the MathJax global
Jan 17, 2018
7d1854f
switch callback to arrow function, passing mathjax
Jan 17, 2018
2145996
used the passed mathjax object instead of the global
Jan 17, 2018
1962ff4
switch div.innerHTML to div.textContent
Jan 17, 2018
6b9c92b
document -> global.document
Jan 17, 2018
41057a8
add ‘mathml’ to the embed types allowd in updateDimensionsEnabled_
Jan 17, 2018
70e7b22
Remove built in mathjax margins, remove extra padding
Jan 17, 2018
ded09a7
remove unused container
Jan 17, 2018
60aea54
remove buildCallback
Jan 17, 2018
7673136
Add amp-mathml.out
Jan 17, 2018
a8c7474
revert yarn.lock
Jan 17, 2018
9dae400
clean up validator-amp-mathml file
Jan 18, 2018
82344f0
use full equality check for github string
Jan 18, 2018
cabddc8
Add an inline example, switch to container layout
Jan 18, 2018
a3a9d94
enable container layout
Jan 18, 2018
d3bc097
remove example width/height
Jan 18, 2018
c518929
only update width if not inlined
Jan 18, 2018
873bb8f
fix up variable names used for resizing
Jan 18, 2018
d9bd1f9
smaller inline example that actually fits inline
Jan 18, 2018
0cd73f6
add amp-mathml.css
Jan 18, 2018
17ae03a
cleanup; proper naming for data width/height
Jan 18, 2018
d5a9f3f
declare: mathml needs css support
Jan 18, 2018
a1bd164
cleanup for linter
Jan 19, 2018
d3adab3
Define types and remove documentation for missing variable to clear g…
Jan 20, 2018
7367908
add new line at end of file
Jan 20, 2018
6353cfe
removed unused isLayoutSizeDefined import
Jan 20, 2018
2dbbdea
remove unused atts in validator-amp-mathml.protoascii
Jan 20, 2018
51fe660
Add inline attr in validator-amp-mathml.protoascii
Jan 20, 2018
6f0728a
new line at end of validator-amp-mathml.protoascii
Jan 20, 2018
c9b9131
clean up tests to match examples
Jan 20, 2018
6e2afd1
Readme cleanup
Jan 20, 2018
0e3dc5f
remove duplicate Attributes line in readme
Jan 20, 2018
0286f09
Add integration test, first pass
Jan 21, 2018
0e9bd9a
cleanup test-mathml.js
Jan 22, 2018
6fdb6e4
tag name should be all caps
Jan 22, 2018
0746c11
Add OWNERS.yaml
Jan 22, 2018
37c39b5
Fix some linting issues
Jan 22, 2018
31cad04
remove integration tests for now
Jan 22, 2018
98d0039
move test and out file to mathml tests folder
Jan 23, 2018
85bf039
remove incomplete/broken test files from PR for now, will add back later
Jan 23, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions 3p/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import {MessageType} from '../src/3p-frame-messaging';
// 3P - please keep in alphabetic order
import {facebook} from './facebook';
import {github} from './github';
import {mathml} from './mathml';
import {reddit} from './reddit';
import {twitter} from './twitter';

Expand Down Expand Up @@ -327,6 +328,7 @@ register('loka', loka);
register('mads', mads);
register('mantis-display', mantisDisplay);
register('mantis-recommend', mantisRecommend);
register('mathml', mathml);
register('mediaimpact', mediaimpact);
register('medianet', medianet);
register('mediavine', mediavine);
Expand Down
68 changes: 68 additions & 0 deletions 3p/mathml.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* Copyright 2017 The AMP HTML Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2018 ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yea; sorry, i started by copying an existing component, I'll clean this up.

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
console.log( 'mathml.js' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't forget to remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:doh:


import {writeScript} from './3p';
import {user} from '../src/log';

/**
* Get the correct script for the gist.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the mathml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, thanks

*
* Use writeScript: Failed to execute 'write' on 'Document': It isn't possible
* to write into a document from an asynchronously-loaded external script unless
* it is explicitly opened.
*
* @param {!Window} global
* @param {string} scriptSource The source of the script, different for post and comment embeds.
*/
function getMathmlJs(global, scriptSource, cb) {
writeScript(global, scriptSource, function() {
cb(global.gist);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should callback with global.MathJax

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right!

});
}

/**
* @param {!Window} global
* @param {!Object} data
*/
export function mathml(global, data) {
user().assert(
data.formula,
'The formula attribute is required for <amp-mathml> %s',
data.element);

getMathmlJs(
global,
'https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.2/MathJax.js?config=TeX-MML-AM_CHTML',
function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use arrow functions and also take in mathjax as param here and use it instead of the global MathJax variable.
e.g.

mathjax => {
...
mathjax.Hub.Queue...
....
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it

// Dimensions are given by the parent frame.
delete data.width;
delete data.height;
const div = document.createElement('div');
div.setAttribute('id','mathmlformula');
div.innerHTML = data.formula;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must be div.textContent = otherwise this crease an XSS vulnerability given formula is not sanitized. textContent should be fine for Latex and Ascii Math. Worht testing for MathML, if it does not work, we unfortunately need to remove support for MathML as input and stick to LaText and ASCII Math since we don't have enough utilities to safely sanitize mathml input.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, ok i'll change and do some testing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far my test cases look fine after the change to textContent, I'll add some more samples from https://developer.mozilla.org/en-US/docs/Mozilla/MathML_Project/MathML_Torture_Test to my example page

document.body.appendChild(div);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

global.document.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will change - what is the context/reason behind global.?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may not be the current window in certain situations where an AMP doc is embedded differently.

MathJax.Hub.Queue( function () {
const rendered = document.getElementById('MathJax-Element-1-Frame');
context.requestResize(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateDimensions is okay in this case ( requestResize is not guaranteed to resize). We make exceptions for updateDimensions per component and this should be fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also mathml needs to be added to updateDimensionsEnabled_() method in ampcontext-integration.js file for it to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, will add

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aghassemi when adding this I noticed on a line I was touching that the check for 'github' isn't using a strict equality check, see https://github.com/ampproject/amphtml/blob/master/3p/ampcontext-integration.js#L72

This should be ===, right? Should I open a separate pull request for this issue? I don't want to introduce unrelated changes here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our JS compiler handles discrepencies between == and === so it is fine in practices, would be nice for consistency, you can change it here in this PR

rendered./*OK*/offsetWidth + 20,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If 20 is for margins, we should leave it up to the developer to set it.I recommend removing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think i was getting cut off display, i'll retest and remove or document reason

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My display was cut off when removing the extra height (https://cl.ly/1G243K2x3W3X) - I tracked this down to same margins built into the formulas rendered by MathJax. I added some code removing them on the fly in 70e7b22. Do I need to add more resiliency here, checking that I found the element before applying styles?

rendered./*OK*/offsetHeight + 20
);
} );

}
);
}
29 changes: 29 additions & 0 deletions examples/amp-mathml.amp.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!doctype html>
<html ⚡>
<head>
<meta charset="utf-8">
<title>amp-mathml example</title>
<link rel="canonical" href="amps.html">
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async custom-element="amp-mathml" src="https://cdn.ampproject.org/v0/amp-mathml-0.1.js"></script>
<script async src="https://cdn.ampproject.org/v0.js"></script>
</head>
<body>
<h2>The Quadratic Formula</h2>
<amp-mathml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all samples now should now only do layout=container

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's also add one sample that's inline in a sentence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

layout="responsive"
width="100" height="100" data-formula="\[x = {-b \pm \sqrt{b^2-4ac} \over 2a}.\]">
</amp-mathml>
<h2>Cauchy's Integral Formula</h2>
<amp-mathml
layout="responsive"
width="100" height="100" data-formula="\[f(a) = \frac{1}{2\pi i} \oint\frac{f(z)}{z-a}dz\]">
</amp-mathml>
<h2>Double angle formula for Cosines</h2>
<amp-mathml
layout="responsive"
width="100" height="100" data-formula="\[ \cos(θ+φ)=\cos(θ)\cos(φ)−\sin(θ)\sin(φ) \]">
</amp-mathml>
</body>
</html>
82 changes: 82 additions & 0 deletions extensions/amp-mathml/0.1/amp-mathml.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/**
* Copyright 2018 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import {isLayoutSizeDefined} from '../../../src/layout';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

import {Layout} from '../../../src/layout';
import {cssstyle} from 'cssstyle';
import {getIframe} from '../../../src/3p-frame';
import { addParamsToUrl } from '../../../src/url';
import { getDataParamsFromAttributes, removeElement } from '../../../src/dom';
import { listenFor } from '../../../src/iframe-helper';

export class AmpMathml extends AMP.BaseElement {

/** @param {!AmpElement} element */
constructor(element) {
super(element);

/** @private {!Element} */
this.container_ = this.win.document.createElement('div');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used, remove

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok


/** @private {?HTMLIFrameElement} */
this.iframe_ = null;
}

/**
* @param {boolean=} opt_onLayout
* @override
*/
preconnectCallback () {
this.preconnect.url('https://cdnjs.cloudflare.com');
}
/** @override */
buildCallback() {
const formula = this.element.getAttribute('formula');
if (!formula || '' === formula) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's have

  user().assert(
       data.formula,
       'The formula attribute is required for <amp-mathml> %s',
       data.element);

here instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that bit is in mathml.js, i removed the buildCallback entirely, it seems superfluous

return;
}
this._formula = formula;
}
layoutCallback () {
const iframe = getIframe(this.win, this.element, 'mathml');
this.applyFillContent(iframe);
// Triggered by context.updateDimensions() inside the iframe.
listenFor(iframe, 'embed-size', data => {
this./*OK*/changeHeight(data['height']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can become
this.element.getResources()./*OK*/changeSize( this.element, newHeight, /* newWidth */ newWidth);

But I believe we should not change width unless it is inline, so give the following a try first:

if(!this.element.hasAttribute('inline') ) {
 // Don't change the width if not inlined.
 newWidth = undefined;
} 
this.element.getResources()./*OK*/changeSize(
        this.element, newHeight, /* newWidth */ newWidth);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, got it. currently my iframes aren't rendering locally so hard to test; this change looks good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works great, thanks!

}, /* opt_is3P */true );
this.element.appendChild(iframe);
this.iframe_ = iframe;
return this.loadPromise(iframe);
}

unlayoutCallback () {
if (this.iframe_) {
removeElement(this.iframe_);
this.iframe_ = null;
}
return true;
}

/** @override */
isLayoutSupported (layout) {
return isLayoutSizeDefined(layout);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return layout == Layout.CONTAINER;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, changed

}

}


AMP.extension('amp-mathml', '0.1', AMP => {
AMP.registerElement('amp-mathml', AmpMathml);
} );
34 changes: 34 additions & 0 deletions extensions/amp-mathml/0.1/test/test-amp-mathml.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Copyright 2018 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import {AmpMathml} from '../amp-mathml';

describes.realWin('amp-mathml', {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for testing, I recommend writing end-2-end integration tests. https://github.com/ampproject/amphtml/blob/master/extensions/amp-sidebar/0.1/test/integration/test-amp-sidebar.js is a good one to use as reference.

In short, the test will essentially render an AMP page with bunch of amp-mathml tags you define. Then inside of the test, use various utilities like poll to wait for those math-ml tags to "render" and "resize". Unfortunately because they are rendered in an iframe, it is not possible to assert that MathML itself rendered but at minimum you can assert that iframe was created and resized to something bigger than 1px.

amp: {
extensions: ['amp-mathml'],
}
}, env => {

let win;
let element;

beforeEach(() => {
win = env.win;
element = win.document.createElement('amp-mathml');
win.document.body.appendChild(element);
});

});
51 changes: 51 additions & 0 deletions extensions/amp-mathml/amp-mathml.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<!--
Copyright 2018 The AMP HTML Authors. All Rights Reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS-IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->

# <a name="`amp-mathml`"></a> `amp-mathml`

<table>
<tr>
<td width="40%"><strong>Description</strong></td>
<td>FILL THIS IN</td>
</tr>
<tr>
<td width="40%"><strong>Availability</strong></td>
<td>FILL THIS IN</td>
</tr>
<tr>
<td width="40%"><strong>Required Script</strong></td>
<td><code>&lt;script async custom-element="amp-form" src="https://cdn.ampproject.org/v0/amp-mathml-0.1.js">&lt;/script></code></td>
</tr>
<tr>
<td class="col-fourty"><strong><a href="https://www.ampproject.org/docs/guides/responsive/control_layout.html">Supported Layouts</a></strong></td>
<td>FILL THIS IN</td>
</tr>
<tr>
<td width="40%"><strong>Examples</strong></td>
<td>FILL THIS IN</td>
</tr>
</table>

## Behavior

FILL THIS IN. What does this extension do?

## Attributes

FILL THIS IN. Does this extension allow for properties to configure?

## Validation
See [amp-mathml rules](https://github.com/ampproject/amphtml/blob/master/extensions/amp-mathml/validator-amp-mathml.protoascii) in the AMP validator specification.
69 changes: 69 additions & 0 deletions extensions/amp-mathml/validator-amp-mathml.protoascii
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
#
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take a look at Contributing Validator Rules. The format for allowing the script tag has been simplified greatly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will do - thanks for the feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the extensions/amp-mathml/validator-amp-mathml.protoascii file in f6d93f3 - I still need to add some valid and invalid test cases

# Copyright 2018 The AMP HTML Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS-IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the license.
#

tags: { # amp-mathml
html_format: AMP
tag_name: "SCRIPT"
spec_name: "amp-mathml extension .js script"
satisfies: "amp-mathml extension .js script"
requires: "amp-mathml"
mandatory_parent: "HEAD"
unique: true
extension_unused_unless_tag_present: "amp-mathml"
attrs: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

name: "async"
mandatory: true
value: ""
}
attrs: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add inline

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

name: "formula"
mandatory: true
}
attrs: {
name: "custom-element"
mandatory: true
value: "amp-mathml"
dispatch_key: true
}
attrs: { name: "nonce" }
attrs: {
name: "src"
mandatory: true
value_regex: "https://cdn\.ampproject\.org/v0/amp-mathml-(latest|0\.1).js"
}
attrs: {
name: "type"
value: "text/javascript"
}
cdata: {
blacklisted_cdata_regex: {
regex: "."
error_message: "contents"
}
}
spec_url: "https://www.ampproject.org/docs/reference/components/amp-mathml"
}
tags: { # <amp-mathml>
html_format: AMP
tag_name: "AMP-MATHML"
satisfies: "amp-mathml"
requires: "amp-mathml extension .js script"
attr_lists: "extended-amp-global"
spec_url: "https://www.ampproject.org/docs/reference/components/amp-hello-world"
amp_layout: {
supported_layouts: RESPONSIVE
}
}
1 change: 1 addition & 0 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ declareExtension('amp-lightbox', '0.1', true);
declareExtension('amp-lightbox-viewer', '0.1', true);
declareExtension('amp-list', '0.1', false);
declareExtension('amp-live-list', '0.1', true);
declareExtension('amp-mathml', '0.1', false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we need a CSS file as well and false becomes true
The CSS file would handle the inline attribute by simply doing

amp-mathml[inline] {
  display: inline-block;
}

Note: we may need to do

amp-mathml {
  height: 1px;
}
amp-mathml[inline] {
  width: 1px;
}

to make it not be 0by0 until resize happens. Try it without the above 1px hacks, if it works all is good. Otherwise if you see mathML not rendering, we may need to add them. (This is because AMP tends to ignore elements that are 0x0)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 1px settings were required, without that the iframe never rendered.

declareExtension('amp-mustache', '0.1', false);
declareExtension('amp-nexxtv-player', '0.1', false);
declareExtension('amp-o2-player', '0.1', false);
Expand Down
27 changes: 27 additions & 0 deletions validator/testdata/feature_tests/amp-mathml.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<!doctype html>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, i have an uncommitted out file that i think was generated when i started, I'll check that and commit it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the file, but I am still missing something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this file to extensions/amp-mathml/0.1/test/validator-amp-mathml.html. The out file will be extensions/amp-mathml/0.1/test/validator-amp-mathml.out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See here for an example extensions test directory with an .html and .out.

<html ⚡>
<head>
<meta charset="utf-8">
<title>amp-mathml example</title>
<link rel="canonical" href="amps.html">
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<style amp-custom>
amp-mathml {
color: red;
}
</style>
<script async custom-element="amp-mathml" src="https://cdn.ampproject.org/v0/amp-mathml-0.1.js"></script>
<script async src="https://cdn.ampproject.org/v0.js"></script>
</head>
<body>
<amp-mathml layout="responsive" formula="\[x = {-b \pm \sqrt{b^2-4ac} \over 2a}.\]">
</amp-mathml>
<h2>Cauchy's Integral Formula</h2>
<amp-mathml layout="responsive" formula="\[f(a) = \frac{1}{2\pi i} \oint\frac{f(z)}{z-a}dz\]">
</amp-mathml>
<h2>Double angle formula for Cosines</h2>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add one with inline

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added, copied over all of items from examples.

<amp-mathml layout="responsive" formula="\[ \cos(θ+φ)=\cos(θ)\cos(φ)−\sin(θ)\sin(φ) \]">
</amp-mathml>
</body>
</html>
Loading