[webkit-reviews] review denied: [Bug 172841] Fix MotionMark to work in Internet Explorer : [Attachment 311844] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 172841: Fix MotionMark to work in Internet Explorer
https://bugs.webkit.org/show_bug.cgi?id=172841

Attachment 311844: Patch

https://bugs.webkit.org/attachment.cgi?id=311844&action=review




--- Comment #6 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 311844
  --> https://bugs.webkit.org/attachment.cgi?id=311844
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311844&action=review

> PerformanceTests/MotionMark/tests/master/resources/image-data.js:42
> +	   // IE taints the canvas context if an SVG image is drawn on it. Any
> +	   // following calls to getImageData will throw CORS security
exception.
> +	   var nameExtension = Utilities.isInternetExplorer() ? "100.png" :
".svg";

It seems like it would be better to have the test be consistent across
browsers, so why not have everyone one use 100.png? 

Also, modern convention says you should do detection, not sniffing, so if you
want to keep the svg in one, png in another, you should replace the
Utilities.isInternetExplorer() check, with a
Utilities.canDrawSVGToCanvasWithoutTainting() check, which does that, and
checks for an exception. That way, if IE changes, it will get the desired
behavior.

That said, I think having everyone do the same thing is better.