Add fallbacks for WP oEmbed JavaScript & CSS

[peterwilsoncc]

Improve JavaScript and CSS support for IE8+, fallbacks include:

• those listed below
• PNG fallbacks for icons (IE9-)
• feature detect select and focus

Original description follows:

---

Add the following to each of the WP oEmbed JavaScript to avoid potential for errors:

• window.load fallback for document.DOMContentLoaded event
• feature detects presence of addEventListener and document.querySelector before attempting to run

I'm in two minds about each of these to be honest (more so the event fallback), will leave to another to decide.

[pento]

For DOMContentLoaded and addEventListener fallbacks, lets do the same as in wp-emoji-loader.js.

querySelector is only a problem for IE < 8. I'd like to at least have some sort of vaguely working fallback - the share dialog working, at a minimum.

[peterwilsoncc]

Replying to pento:

querySelector is only a problem for IE < 8. I'd like to at least have some sort of vaguely working fallback - the share dialog working, at a minimum.

Coolio, can do. A few questions:

• SVG share icons will need a fallback for IE < 9, text, external PNG or inline PNG?
• Any objection to adding in some IDs for the share dialog fallback? I'd like to avoid selecting a tonne of elements and checking className.

Note to self: feature check focus() in refresh.

[pento]

Replying to peterwilsoncc:

• SVG share icons will need a fallback for IE < 9, text, external PNG or inline PNG?

I'm inclined to make them external. No point in sending them down the wire for sites that won't use them.

• Any objection to adding in some IDs for the share dialog fallback? I'd like to avoid selecting a tonne of elements and checking className.

Go for it.

[peterwilsoncc]

There is some additional feature detection & error checking I need to do to expand browser support. I will upload refresh Sat or Sunday evening USA time.

@melchoyce are you able to upload some PNG fallbacks for the icons?

I've modified the the description and title to reflect browser support rather than specific JS support.

[melchoyce]

Added comment + share icons in embed-icons.zip​. Forgot about the X, so I'll upload that next.

[melchoyce]

Added x.png​

[peterwilsoncc]

Switching the type to bug as the JS was throwing errors in supported browsers.

Refreshed patch added in 34204.2.diff​:

• embed.zip​ new images to be extracted to the directory src/wp-includes/images/embed/
• in the iframe:
  • cross browser events - dialog and tabs work back to IE7
  • removed use strict, this in the event methods generates warnings in jshint.
  • feature detection for focus & select
  • the dialog uses IDs and tags for getting all elements required for opening/closing the dialog
  • inputs use querySelectorAll and does not add the focus/select events in browsers without support
  • styles work back to IE7 but IE7 is ugly. IE8 is surprisingly good.
• in the embedding JavaScript (window.top):
  • JS support only available in browsers supporting querySelectorAll and modern events.

[adamsilverstein]

@peterwilsoncc I tested this with Browserstack, IE8:

• no more JS errors
• I see the share button png images now and hover works
• clicking share, nothing happens. no JS errors in console: ​http://cl.ly/image/152q0w2C2x2x

Did I miss anything? Share should work in ie8, right?

[ocean90]

Can't we just show text ("x Comments", "Share", "Close") if the browser doesn't support SVGs?

[wonderboymusic]

In 35083:

oEmbed: add fallback icons (not used yet) for older browsers.

Props melchoyce.
See #34204.

[peterwilsoncc]

Replying to adamsilverstein:

Did I miss anything? Share should work in ie8, right?

I'll do some further testing to see what I missed. Cross-browser events and a generic selector might need to be added to the front end too.

If this increases the JS too much then we can feature detect and hide the share icons by expanding on 34204.1.diff.

[peterwilsoncc]

Ok, so hands up if your forgot the entire point of the security attribute was to prevent JS running in older browsers :)

Revised approach:

• share icon only displays if JavaScript is running in the iframe
• security policy locks out browsers without querySelector/modern events so reverted to feature test in patch one
• css fallbacks still in play for the browser impaired

@ocean90

There's not a reliable feature detect available, ​see Modernizr discussion , but it an approach worth considering. Inline SVG with text fallbacks may be the ticket.

[swissspidy]

Thanks for digging into all this!

+1 for only showing the share icon if JavaScript is working.

Some minor notes:

• wp-oembed-embed-ie.css is missing from your latest patch
• Can you move wp_enqueue_style( 'open-sans' ); to wp_oembed_enqueue_scripts too?
• no-js class should always be removed (or better: renamed to js, regardless of featuredBrowser
• Is featuredBrowser even needed? It's always true since the JS isn't run in IE8 and lower.

[peterwilsoncc]

Apologies for the missed files, refreshed in 34204.4.diff​.

The no-js/js classes indicates whether the browser passes the feature tests & can add events for the popup. It should only change if the features pass.

Perhaps the answer is better class names:

• queryselector-(un)supported
• w3events-(un)supported

Please make suggestions, naming things is hard.

~

featuredBrowser is required in the event a visitor hits the embed HTML directly, eg ​http://wordpress-develop.dev/?p=1&embed=true

~

Enqueueing open-sans moved.

~

If this is committed prior to #34272, #34272 will require a refresh.

[swissspidy]

Ah, I didn't see you were already replacing no-js with js. I think that should stay, adding the sharing icon when the class is js. So that's fine.

What I'd change now:

• Move the no-js / js class to the body class, like in the rest of core
• Rename featuredBrowser to something like supportedBrowser.
• Hook print_emoji_detection_script, print_oembed_embed_scripts et al to the new wp_oembed_enqueue_scripts hook.

I can do the refresh once these tickets get in.

[peterwilsoncc]

Refresh following [35235] and the following changes in 34204.5.diff​.

• featuredBrowser => supportedBrowser
• enqueue scripts hook renamed enqueue_embed_scripts to match new convention
• Moved the print_emoji_styles hook to print_embed_styles
• print_emoji_detection_script kept on embed_head to follow convention from wp_head
• in both twentyfifteen and -sixteen the (no-)js classes are on the HTML element so I have left it there. I can't see any other instances of use on the front end.

Note: there was a bug created following #34272 so the 34272.2.diff:ticket:34272​ will need to be applied/committed for this patch to work.

[swissspidy]

There's no print_embed_styles hook.

print_emoji_styles is already hooked to wp_print_styles so the styles are already loaded.
34204.diff​ correctly hooks enqueue_embed_scripts to embed_head instead of oembed_head.
Also, now that I'm working on it, we should leave the other hooks as-is.

Thanks for the info re: the no-js class. In the admin it's added to the body, but I think here it's fine on the html embed.

[peterwilsoncc]

Replying to swissspidy:

There's no print_embed_styles hook.

print_emoji_styles is already hooked to wp_print_styles so the styles are already loaded.
34204.diff​ correctly hooks enqueue_embed_scripts to embed_head instead of oembed_head.

Thanks, missed both of these.

Looks like there is both print_embed_styles L451 and wp_print_styles L454 on the embed_head hook. Should this be the case or should one be removed?

Also, now that I'm working on it, we should leave the other hooks as-is.

Thanks for the info re: the no-js class. In the admin it's added to the body, but I think here it's fine on the html embed.

Cool, now I see where you were coming from. Ta.

[swissspidy]

print_embed_styles is more like the equivalent to print_emoji_styles, not wp_print_styles. Hooking print_embed_styles to wp_print_styles would be possible, but change the order in which the CSS is loaded.

I prefer the default CSS to be loaded before any other CSS, as it currently is the case.

[swissspidy]

If nothing's holding us back here, I'd like to get this in.

[peterwilsoncc]

Note: #34451 will require a refresh if this is committed first.

Is there anything else this ticket needs to cover before being committed?

[pento]

34204.6.diff​ updates the patch to apply cleanly against trunk.

I'm not particularly excited about the * IE7 hack being in wp-embed-template.css, I think browser hacks should really be kept to conditionally loaded files.

@helen: Do you have opinions on this?

[swissspidy]

Replying to pento:

34204.6.diff​ updates the patch to apply cleanly against trunk.

I'm not particularly excited about the * IE7 hack being in wp-embed-template.css, I think browser hacks should really be kept to conditionally loaded files.

Note: In that patch the * hack is now in both CSS files. Moving it to wp-embed-template-ie.css makes sense, if that hack is really needed. wp-includes/dashicons.css doesn't have these hacks in it.

[helen]

The short answer is no, don't keep them anywhere at all, but I'd like to know what those hacks are there for, exactly, please.

[peterwilsoncc]

Replying to helen:

The short answer is no, don't keep them anywhere at all, but I'd like to know what those hacks are there for, exactly, please.

The CSS fallbacks are less of an issue now the embed template in themes is blessed, refer #34278.

The lte ie8 CSS is there to provide a fallback for the SVG elements

The * hacks are there to cover a number of issues:

• IE7 does not support data-uris, using the * hack avoids the extra HTTP requests for IE8.
• display:table, etc is not supported in IE7- so the site icon, site name and icons were stacked
• without the * hacks, IE7 does not display the reference to comments in the inline-block

I'm happy to discard the * hacks or to move them all to the IE file.

[pento]

34204.7.diff​ refreshes the patch against trunk.

@peterwilsoncc - could you try moving all of the * hacks to the IE file? it's easier to judge which are worth keeping if they're all in one place.

[peterwilsoncc]

@pento moved to the IE file in 34204.8.diff​, other notes:

• the cssmin upgrade deletes * hacks, added an option to prevent.
• At some point .js .dashicons-share was set to block, it should be inline-block so this has changed.
• Most of the * hacks are to emulate inline-block in IE7
• .wp-embed { *width: auto; } is to account for the lack of border-box support in IE7
• and the png fallbacks this ticket started with

[peterwilsoncc]

As discussed, 34204.9.diff​ is the * hacks be gone edition.

Width definition is removed from wp-embed as it was benign without an explicit width set on the body.

[pento]

In 35466:

Embeds: Add fallbacks for IE7-9.

Older IE versions need just that little bit of extra tender care to keep them going.

Props peterwilsoncc, swissspidy, pento.

Fixes #34204.

[pento]

In 35467:

Embeds: Revert [35083], as the PNG files ended up not being used in [35466].

See #34204.