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

RESOLVED FIXED in Firefox 36

Status

defect
RESOLVED FIXED
5 years ago
Last year

People

(Reporter: pbro, Assigned: harth)

Tracking

(Depends on 1 bug, Blocks 1 bug)

unspecified
Firefox 36
Dependency tree / graph

Firefox Tracking Flags

(e10s+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
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).
Duplicate of this bug: 1047576
Duplicate of this bug: 1067139
Depends on: 1087807
Assignee: nobody → fayearthur
Posted patch WIP (obsolete) — Splinter Review
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
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.
Attachment #8517814 - Attachment is obsolete: true
Attachment #8518603 - Flags: review?(mratcliffe)
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.
Attachment #8518603 - Flags: review?(mratcliffe) → review+
(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.
Thanks. I had no reason for the jsm, I was just copying code.

Try:
https://tbpl.mozilla.org/?tree=Try&rev=e8624af11648
Attachment #8518603 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a6775c581217
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
Blocks: 1097195
This bug patch is preventing to enter any accented letter like "ê" or "ë". Removing it fixes bug 1097195 I reported.
Bugday-20141217- Eye-dropper toolbar icon picks color on click and saves the picked color on inspector.
Depends on: 1176595
Depends on: 1301535
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.