ampproject / amp-wp Goto Github PK

This is the request we're making to Google Fonts:
https://fonts.googleapis.com/css?family=Merriweather|Open+Sans:400,700,400italic,700italic
https://github.com/Automattic/amp-wp/blob/master/template.php#L8

I believe we meant to call the 700 weight and italic styles of Merriweather, not Open Sans. I'm seeing faux bold and faux italic when I view an AMP page in a mobile browser.

Replace the SVG logo with the site icon if one is available and allow the color of the bar to be specify in the appearance customizer.

• Show Site Icon, if set (instead of WP logo)
• Show header image, if set

To avoid users having to take a manual step.

I'm implementing a way to turn AMP off for select posts. Default would be on, but a check that a meta value of 'amp_enabled' exists and that value is false will result in a pass.

What do y'all think?

https://github.com/Automattic/amp-wp/compare/master...mterenzio:mterenzio-patch-1?expand=1

For WPCOM + Jetpack: we should output a stats pixel to bump appropriate stats when an AMP page is viewed.

See https://github.com/ampproject/amphtml/blob/master/builtins/amp-video.md

WPCOM + Jetpack: Should handle VideoPress as well.

See https://github.com/ampproject/amphtml/blob/master/extensions/amp-twitter/amp-twitter.md

The layout attribute spec was updated: https://github.com/ampproject/amphtml/blob/master/spec/amp-html-layout.md#layout -- we should conform to the new guidelines.

e.g. right now, we apply layout=responsive to almost all images, which causes them to stretch to fill in the space when they are smaller than the viewport and that looks terrible.

We should make it easier to swap in your own template to allow for customization of styles, layout, header/footer, etc.

So when it's toggled on all your AMP-enabled content links redirects to the AMP version of the content if the visitor is on a smartphone, but the links remain the same for non-smartphone users. This way visitors can take advantage of the speed increases even though they didn't access your content through Google's new AMP-powered carousel. If it's toggled off (default), then all your links point to the original content, even for smartphone visitors.

Iframes and links from youtube are not working in my amp post. Showing this console error
"Expected width to be available and be an integer/length value: null"
"Blocked script execution in 'youtube link' because the document's frame is sandboxed and the 'allow-scripts' permission is not set"

Custom Post Types should be able to opt-in to AMP support.

https://github.com/ampproject/amphtml/tree/master/extensions/amp-anim

Why don't we just use the default content filter from WordPress core instead of using remove filters?

See https://github.com/ampproject/amphtml/blob/master/extensions/amp-instagram/amp-instagram.md

amp_post->get_author_avatar() returns an <Img/> tag. This should be an <amp-img/> tag instead.

A Vine video is described in an AMP template as <amp-vine vineid="">

A Vine video may appear in WordPress content as an oEmbed-compatible URL. Vine is a supported oEmbed provider as of WordPress 4.1.

A Vine video may be specified in a shortcode as [vine url=""] or [vine id=""]

• WordPress.com Vine docs
• AMP template component

Not all <img> tags have width/height attributes which are required for the matching <amp-img> tag. We should find a way to include that somehow.

• If srcset or sizes are included, try to pull from them, if possible.
• Pull from attachment metadata.
• Fetch the remote image and calculate/save the size (getimagesize and wpcom's getimagesize).

Related #13

Maybe you should consider the option to add a .editorconfig, that should be helpful for somebody else to stick to the same standard you are using for this project and *.php files.

Thank you ! ✋

When you have the plugin installed, there's no context as to what AMP is. Maybe change the description to something like "Enable Accelerated Mobile Pages https://www.ampproject.org/ on your WordPress site."

Need to remove PHP 5.3-isms like anonymous functions.

Need to also test against Jetpack :)

Just tried the plugin on http://www.get-business-online.com/ and saw the following:

1. Static front page is not handled properly. When adding "amp/" to my home page, you get a 404
2. When adding ?amp=1 to the home page, you get the blog page instead, apparently unchanged
3. Links on AMP pages should have "amp/" to them. We can't expect mobile users to add that manually
4. Pages and posts show with some standard theme and no navigation
5. Sidebars seem to be ignored
6. Sharing widget styling seems to be ignored and sharing links appear as simply links
7. Permalinks must be updated after activation to make the "amp/" option work
8. Adding "amp/" to a post category URL gives a 404
9. Adding "?amp=1" to a post category URL shows the page unchanged

This plugin is seriously in its early stages.

Cheers,
Gal

A lot of pages that use custom templates may not have a conventional IMAGE/TITLE/CONTENT structure. The ability to prevent those pages from generating what would essentially be empty AMP pages would be useful.

See https://github.com/ampproject/amphtml/blob/master/extensions/amp-youtube/amp-youtube.md

This is because it adds stylesheet to the head of the page. That can be easily fixed with define( 'ICL_DONT_LOAD_LANGUAGE_SELECTOR_CSS', true); when rendering the AMP version of the page.

ampproject/amphtml#385

We need a min-width or drop the responsiveness. Since floated images more than likely don't need to be responsive anyway, and it's hard to set a minimum min-width for all floated images.

Basically, any images with the class alignleft or alignright should drop layout=responsive

See https://github.com/ampproject/amphtml/blob/master/extensions/amp-iframe/amp-iframe.md

Should hopefully catch all other shortcodes/oEmbed that aren't natively supported in AMP.

I noticed that this plugin causes both the Floating Social Bar and the Sexy Author Bio plugin's CSS code to be rendered on the page as text. Also, there is a JavaScript conflict with the WP Tab Widget plugin. I had to disable the plugin in order to get the JavaScript code to not render on the page as text. My site is hosted on WP Engine, but it doesn't appear to be anything specific to their services (at least that I can tell). For now I'm just hiding the display of the CSS text using display: none in CSS, ironic and horrible so if anyone knows a better way to fix this please let me know. I can then update my Sexy Author Bio plugin accordingly.

Some PHP5.3+isms have snuck in and we need to remove them.

e.g. don't need to add iframe script to all amp pages.

Using 0.1.

Photon enabled this first working block is what my typical image HTML looks like. The second block is with AMP. The image URLs are a bit different, but both are valid.

Console:

Works:
<img class=" size-full wp-image-2202 alignleft" src="http://i0.wp.com/developdaly.com/shared/content/uploads/2015/05/Screen-Shot-2015-05-12-at-1.38.51-PM.png?zoom=2&amp;resize=115%2C36" alt="Insecure https" width="115" height="36" srcset="http://i0.wp.com/developdaly.com/shared/content/uploads/2015/05/Screen-Shot-2015-05-12-at-1.38.51-PM.png?zoom=2&amp;resize=115%2C36" src-orig="http://i0.wp.com/developdaly.com/shared/content/uploads/2015/05/Screen-Shot-2015-05-12-at-1.38.51-PM.png?resize=115%2C36" scale="2">

Doesn't work:
<amp-img class=" size-full wp-image-2202 alignleft -amp-element -amp-layout-container -amp-error" src="http://i0.wp.com/developdaly.com/wp/../shared/content/uploads/2015/05/Screen-Shot-2015-05-12-at-1.38.51-PM.png?resize=115%2C36" alt="Insecure https" id="AMP_2"><img amp-img-id="AMP_2" alt="Insecure https" class="-amp-fill-content -amp-replaced-content" width="0" height="0"></amp-img>

http://developdaly.com/seriously-banks-obvious-website-security-issues-at-major-us-banks/
vs
http://developdaly.com/seriously-banks-obvious-website-security-issues-at-major-us-banks/amp/

The most important problem is that the pages generated by this plugin do NOT show the Google ads: here is a page of Monty Python quotes NOT generated by this plugin, and here is the same page generated by this plugin. As you can see from the links, plugin strips Google ads and replaces them with pure code.

The second problem concerns the usability of this plugin. It does NOT add "amp" at the end of existing pages. Rather, you can create those links yourself. However, how can a site with hundreds of pages and thousands of links manually replace every link with the version that has 'amp' at the end of itself? This is impossible. Therefore, this plugin has to automatically find and replace instances of existing links. Otherwise, Google's bots are not going to index AMP pages.

On WPCOM, we should set longer-ish expiries, similar to how we handle feeds.

Ideally, these should invalidate on post updates.

After all known components have been converted to amp versions, we should strip out anything that is not supported:

script, noscript, style, frame, frameset, object, param, applet, form, input, button, textarea, select, option

For attribues, need to strip all style attributes and anything that starts with on.

See: https://github.com/ampproject/amphtml/blob/master/spec/amp-html-format.md#html-tags

Protected Embeds are kinda weird (they rely on a javascript) and we probably need some workarounds for them.

I just wanted to let you know that, I been testing the amp-wp plugin, and that the only conflict that I have noticed so far is with the AddThis Sharing Buttons plugin - when the AddTis plugin is active the amp page will include the javascript code - as can bee seen here - http://torbjornzetterlund.com/wordpress-plugin-for-google-accelerated-mobile-pages-project/amp/

Scroll down to the end and javascript code is shown on the page.

If post content has something like <script>alert( 'Howdy' )</script>, running the content through kses for amp rendering results in alert( 'Howdy' ) showing up in the content, which isn't ideal.

See https://github.com/ampproject/amphtml/blob/master/extensions/amp-audio/amp-audio.md

See https://github.com/ampproject/amphtml/blob/master/extensions/amp-carousel/amp-carousel.md

For consistency and less reliance on regexes.

See https://markjaquith.wordpress.com/2014/02/19/template_redirect-is-not-for-loading-templates/

Makes sense not to include comments in the amp version, of course, but if comments are on, at least a link to them seems like it would be a good idea?

www.example.com/article-permalink/amp/ doesn't work until rewrite flushing is manually invoked (from settings) while www.example.com/article-permalink?amp=1 works just fine.

Improve the overall styles for the current template.

In this blog post: http://www.chrisfinke.com/2015/09/28/and-some-benches/amp/ the images are squished to about 60px tall but are still the full width of the column.

amp-video only supports <source> <div fallback> and <div placeholder> as children.

amp-audio only supports <source> and <div fallback>.

We should reformat the child nodes to match this.

We should probably have default sizes for amp elements that do not have width or height or both set.