1295607 - The eyedropper is broken at "https://www.mozilla.jp/about" page (CSP errors)

Status: VERIFIED FIXED

(DevTools :: Inspector, defect)

Description

[magicp]

Attachment: eyedropper-is-broken-at-www.mozilla.jp_about.png

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160816030459

Steps to reproduce:

1. Start Nightly
2. Go to "https://www.mozilla.jp/about"
3. Open DevTools > Inspector
4. Run Eyedropper


Actual results:

The eyedropper is broken.


Expected results:

The eyedropper works at "https://www.mozilla.jp/about" page

Comment 1

[magicp]

Regression range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=e0bc88708ffed39aaab1fbc0ac461d93561195de&tochange=a7a882d122e36defe6c2a102a28ae7dfc16311c5

Comment 2

[Patrick Brosset <:pbro>]

This site defines a CSP header:

"default-src 'self'; connect-src 'self' https://*.typekit.net; script-src 'self' https://use.typekit.net https://www.google-analytics.com https://maps.googleapis.com https://amara.org; style-src 'self' 'unsafe-inline' https://use.typekit.net; font-src 'self' data: https://use.typekit.net; child-src https://www.youtube-nocookie.com https://www.youtube.com https://amara.org; img-src *; media-src *;"

The console shows the following error:

Content Security Policy: The page’s settings blocked the loading of a resource at data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAABb4AAAGhCAYAAACu1KIlAAAgAElEQVR4nOzd+Z8U1b0//vtPfL6fm6g3Ny7RmGiM7Y6KCppBQWGUxRVkUXYEN1xANgU0N4kJeo2SRIJbSOLVXCMiGBWBmd67q2vv7pkBhlwFBYzm/sLn8Xh9f6h6nz7V9MCANTA9vOrxqMd013LOeb5PVePjXcdT/wIuXLhw4cKFCxcuXLhw4cKFCxcuXLhw4cKlnyz/8i//8s3X443gwoULFy5cuHDhwoULFy5cuHDhwoULFy5cZIkl8V3MbYFZ2AYjvxV2KQnXTKKQ2QwjvxWOmYRttKOY2wLbaIeR3wqzsK22rRTst4324HP43chvhVlsg1nYFhxfSsIxkyjmtsAxk6oMs9gGx0yqcs1iG2yjHW64zchvDY4Ly5E6zWIb3LAcOZcOOuiggw466KCDDjrooIMOOuhoMkcxhZ07qjhw4IBKdhw4cIAr1x5X/TrZ3lWGUUwN3PuDDjpOYEcsiW/PyqgAGPmtKgiOGTSgHi8N8KxMDR8eZxa2RVFhQMxiWySoZrENhcxmVaZZbIuUEfkeBk+2SdD1IEl76KCDDjrooIMOOuiggw466KCDjiZ0GGl8uf+L4zGokEsTLvv27kaxkDxx7g866DgBHfGN+A4Ll0r1huXTH6OQ2RwFhjCzsE01SgW1pAVUPhvtcK2UOka2ST2umVRBl3Jto/YkQC9LynbM2hMI1WF00EEHHXTQQQcddNBBBx100EFH8zqMNPbt3XO886pc+umyb+8emEb6xL0/6KDjBHLEOtWJNEAqNIttKsNuFrYpgKDkyYEAi7kt6mmAwgi+sA2OWTcUXqtLD6q+X9COGT1G2mOXkvDtjGoPHXTQQQcddNBB
        
When the eye-dropper is used, it gets a screenshot of the page via a canvascontext.drawWindow, and then loads it as an image, using new Image(). Creating the image is probably what triggers the CSP error and therefore prevents the eyedropper from being initialized correctly.

http://searchfox.org/mozilla-central/rev/03b3c20a6ec60435feb995a2a23c5926188c85a1/devtools/server/actors/highlighters/eye-dropper.js#183-199

Comment 3

[Patrick Brosset <:pbro>]

I think this used to work before because we weren't creating the image in the content window, but in a different chrome window we used to display the eyedropper window:
https://dxr.mozilla.org/mozilla-beta/rev/a7839743d3854db94d7c5213e030890638f77587/devtools/client/eyedropper/eyedropper.js#231

Comment 4

[Patrick Brosset <:pbro>]

Investigating other ways to create the image data we need, without causing CSP errors.

Comment 5

[Patrick Brosset <:pbro>]

Attachment: Bug 1295607 - Avoid CSP errors when drawing the window into the eyedropper;

Pages defining CSP response headers used to be a problem for the eyedropper.
Indeed, the eyedropper would take a screenshot of the window with
canvas.drawWindow and then load the resulting data as an Image. But in order
to access the Image() constructor, it would use the content window:
new window.Image(), and that wasn't possible with CSP headers.

With this change, the eyedropper creates an ImageBitmap with
window.createImageBitmap() and that doesn't cause CSP errors, and still works
fine because ImageBitmap are consumable by the eyedropper.

This change also adds a new test to prevent this bug from coming back.

Review commit: https://reviewboard.mozilla.org/r/72614/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72614/

Comment 6

[Patrick Brosset <:pbro>]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=257b0f0bfe32&group_state=expanded

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8782439 [details]
Bug 1295607 - Avoid CSP errors when drawing the window into the eyedropper;

https://reviewboard.mozilla.org/r/72614/#review70434

createImageBitmap must have been difficult to find... great job!

Do we have the same issue with our screenshot buttons?

Comment 8

[Patrick Brosset <:pbro>]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #7)
> Do we have the same issue with our screenshot buttons?
I've checked and we actually don't. That's probably because once we get the image data from the content window, we save it to a file, we don't need to load it as an image inside the content window again like we do for the eyedropper highlighter.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/5a3b356bd3fb
Avoid CSP errors when drawing the window into the eyedropper; r=miker

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5a3b356bd3fb

Comment 11

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 51.0a1 (2016-08-16) on Windows 7 , 64 Bit!

This bug's fix is verified with latest Nightly 
 
Build   ID  20160824030337
User Agent  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:51.0) Gecko/20100101 Firefox/51.0

Comment 12

[magicp]

verified in 51. uplift to aurora?

Comment 13

[Patrick Brosset <:pbro>]

Comment on attachment 8782439 [details]
Bug 1295607 - Avoid CSP errors when drawing the window into the eyedropper;

Approval Request Comment
[Feature/regressing bug #]: bug 1262439
[User impact if declined]: The eyedropper was completely rewritten in FF50, and some regressions (including this one) were filed as a result. This one in particular prevents the new eyedropper from working at all if the page currently displayed in the tab defines CSP headers.
So if we decline this uplift, then people trying to pick a color from the page with the eyedropper when the page has CSP won't be able to do it.
[Describe test coverage new/current, TreeHerder]: New automated mochitest added, also manually tested in nightly, and verified by the person who opened the bug.
[Risks and why]: Not as minor as the other eyedropper uplifts I requested because there is slightly more code involved. But still, similarly to the others, this is devtools only code and more specifically, eyedropper only code. So it's never going to impact the browser UI itself, only the eyedropper, if there were bugs in this patch, which is unlikely given that it's been tested.
[String/UUID change made/needed]: None

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8782439 [details]
Bug 1295607 - Avoid CSP errors when drawing the window into the eyedropper;

Fix was verified in Nightly, Aurora50+

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This hit conflicts trying to uplift to aurora, can we get a rebased patch?

Comment 16

[Patrick Brosset <:pbro>]

Attachment: rebased-for-aurora.diff

This should work on Aurora now.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/06a16c8ce554

Comment 18

[magicp]

verified in 50 (20160901004001) Thanks!