1040653 - [e10s] Eye-dropper tool doesn't work with e10s windows

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Patrick Brosset <:pbro>]

STR:

- Open a new e10s window
- Open the inspector
- Make sure you have the eye-dropper toolbar icon visible in the toolbox (if not, go to the settings tab, and check the "grab a color from the page" option in the "available toolbox buttons" section)
- Click on the eye-dropper icon
- Move your mouse over the page

Expected: the eye-dropper shows a magnified view of the page and allows to pick the color of one of the pixels on click.

Actual: the eye-dropper remains white and only that color can be picked on click.

Comment 1

[Patrick Brosset <:pbro>]

The problem is that the eye-dropper, being a toolbox-side only tool, doesn't have any code that runs in the content process, and there are probably things it does that can't cross process boundaries.

I think the eye-dropper should load a frame script in the content process and use it for communication with the page (see https://developer.mozilla.org/en-US/docs/The_message_manager).

Comment 4

[Heather Arthur [:harth]]

Attachment: WIP

A patch. Though I got the tests passing, I haven't manually tested on Win/Linux yet, so I'll have to do that. https://tbpl.mozilla.org/?tree=Try&rev=dd15808f42ef

Comment 5

[Heather Arthur [:harth]]

Attachment: Use frame script to get content screenshot + fix tests and enable on e10s

Tested on Win+Linux and fixed Linux pixel offset bug.

With this patch, the eyedropper should work the same as before, except for a minor difference when you're inspecting the very edge of the content/chrome border. It will show a blank white area where the chrome is, or vice versa. depending on what side you're on. It's not very noticeable, and still shows the correct color for whatever pixel you're inspecting.

We fetch a screenshot of the content from a frame script, and use that when we're inspecting a content pixel. This makes opening it asynchronous, so that forces some changes in the tests.

Comment 6

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

Comment on attachment 8518603 [details] [diff] [review]
Use frame script to get content screenshot + fix tests and enable on e10s

Review of attachment 8518603 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/eyedropper/eyedropper-child.jsm
@@ +1,4 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> +* License, v. 2.0. If a copy of the MPL was not distributed with this
> +* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +

Just a nit really so feel free to ignore it.

Not sure why you created a jsm here instead of a js file... don't forget that the reload() command only works with js files.

Comment 7

[Patrick Brosset <:pbro>]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #6)
> Just a nit really so feel free to ignore it.
> 
> Not sure why you created a jsm here instead of a js file... don't forget
> that the reload() command only works with js files.
I agree with Mike, our other frame scripts are js files, not jsms.

Comment 8

[Heather Arthur [:harth]]

Attachment: e10s eydropper for check in

Thanks. I had no reason for the jsm, I was just copying code.

Try:
https://tbpl.mozilla.org/?tree=Try&rev=e8624af11648

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/a6775c581217

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a6775c581217

Comment 11

[Frederic Bezies]

This bug patch is preventing to enter any accented letter like "ê" or "ë". Removing it fixes bug 1097195 I reported.

Comment 12

[pratyasmitamishra]

Bugday-20141217- Eye-dropper toolbar icon picks color on click and saves the picked color on inspector.