965037 - Fix PluginDetect_Flash.js to not use eval()

Status: RESOLVED FIXED

(Webmaker Graveyard :: Popcorn Maker, defect)

Description

[Jon Buckley [:jbuck]]

https://github.com/mozilla/popcorn.webmaker.org/blob/master/public/external/PluginDetect/PluginDetect_Flash.js is using eval() to test whether the browser is IE or not with the following line:

$.isIE = eval("/*@cc_on!@*/!1");

It'd probably be best to use an updated version of the script at http://www.pinlady.net/PluginDetect/scripts/PluginDetect_Flash.js and patch it to remove eval() or use a different flash detection script that doesn't require eval().

Comment 1

[Alexander [:admix]]

Can it be written as: 
$.isIE = JSON.parse("/*@cc_on!@*/!1"); ?

Comment 2

[Jon Buckley [:jbuck]]

I don't think so, because IE's condition compilation doesn't work in JSON: http://msdn.microsoft.com/en-us/library/8ka90k2e%28v=vs.94%29.aspx

I don't even understand why they use conditional compilation to detect IE...

Comment 3

[Alexander [:admix]]

Can you assign this one to me please, I need to get it done anyways to get to CSP. Hopefully I will figure it out.

Comment 4

[Jon Buckley [:jbuck]]

Another option here might be replacing that plugin detection script with our own. For modern browsers we can look at navigator.plugins, but we'll need to figure out something that'll be compatible with IE9 and IE10 too.

Comment 5

[Alexander [:admix]]

And how about this solution:
isIE = (function () {
  var tmp = document.documentMode, e, isIE;

  // Try to force this property to be a string.
  try{document.documentMode = "";}
  catch(e){ };

  // If document.documentMode is a number, then it is a read-only property, and so
  // we have IE 8+.
  // Otherwise, if conditional compilation works, then we have IE < 11.
  // Otherwise, we have a non-IE browser.
  isIE = typeof document.documentMode == "number";

  // Switch back the value to be unobtrusive for non-IE browsers.
  try{document.documentMode = tmp;}
  catch(e){ };

  return isIE;
}());

link: http://www.pinlady.net/PluginDetect/IE/

Comment 6

[Alexander [:admix]]

Jon, can you take a look at my solution when you have time. Probably it will work, as popcorn doesn't work for IE < 8 .. Thanks. Or should I do the PR and we will look how it in practice works?!

Comment 7

[Jon Buckley [:jbuck]]

Looking at the beautified code, lets update to the latest version of the flash detection script and http://www.pinlady.net/PluginDetect/scripts/PluginDetect_Flash.js and just delete the string "||eval("/*@cc_on!@*/!1")".

Comment 8

[Alexander [:admix]]

Attachment: https://github.com/mozilla/popcorn.webmaker.org/pull/512

Updated to version 0.8.7; Removed eval()
-> https://github.com/mozilla/popcorn.webmaker.org/pull/512

Comment 9

[Alexander [:admix]]

Comment on attachment 8392304 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/512

Put back that requires block.
PR -> https://github.com/mozilla/popcorn.webmaker.org/pull/512

Comment 10

[[github robot]]

Commits pushed to master at https://github.com/mozilla/popcorn.webmaker.org

https://github.com/mozilla/popcorn.webmaker.org/commit/ec0f32f749d2c8e1d5bc7a170b053abcb6f74729
[bug965037] - Updated PluginDetect_Flash.js to v. 0.8.7 and removed eval()

[bug965037] - Put back requirejs shim

https://github.com/mozilla/popcorn.webmaker.org/commit/513295c518fe7634f5f236de9dab7f657cf35932
Merge pull request #512 from admix/bug965037

[bug965037] - Updated PluginDetect_Flash.js to v. 0.8.7 and removed eval()