magicp Reporter
	Description • 8 years ago

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

Steps to reproduce:

1. Start Nightly
2. Open DevTools > Browser Toolbox (Ctrl+Alt+Shift+I)
3. Enable new eyedropper


Actual results:

Cannot use new eyedropper in the Browser Toolbox


Expected results:

Can we use new eyedropper in the Browser Toolbox? If it is not, should disable(display: none) it. If it can be used, works well.

	magicp Reporter
	Comment 1 • 8 years ago

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

	Julian Descottes [:jdescottes]
	Comment 2 • 8 years ago

Recent regression from eyedropper migration. Eyedropper cannot be used in Browser Toolbox -> P1

Inspector bug triage, filter on CLIMBING SHOES.

	Albert Scheiner [:alberts]
	Comment 3 • 8 years ago

Is this the reason there is not color picker anymore in the latest Nightly 51.0a1 (2016-08-06) and Dev Edition 50.0a2 (2016-08-06)?

	Julian Descottes [:jdescottes]
	Comment 4 • 8 years ago

(In reply to Albert Scheiner [:alberts] from comment #3)
> Is this the reason there is not color picker anymore in the latest Nightly
> 51.0a1 (2016-08-06) and Dev Edition 50.0a2 (2016-08-06)?

Unlikely, could you file a separate bug with detailed STRs ?

	Albert Scheiner [:alberts]
	Comment 5 • 8 years ago

(In reply to Julian Descottes [:jdescottes] from comment #4)
> (In reply to Albert Scheiner [:alberts] from comment #3)
> > Is this the reason there is not color picker anymore in the latest Nightly
> > 51.0a1 (2016-08-06) and Dev Edition 50.0a2 (2016-08-06)?
> 
> Unlikely, could you file a separate bug with detailed STRs ?

It seems like a found the reason. On my FF 50.0a2 (2016-08-07)/Win I just found out that the color picker got moved from the Tool icon bar in the top right corner into the Inspector window's top bar next to the search input. It (therefore!?) also got removed from the "Toolbox Options" menu - where I was looking for the setting, too.

... found the task bug 1260225 :)

	Albert Scheiner [:alberts]
	Comment 6 • 8 years ago

... and I just saw that I was a a bit confused regarding "color picker" and "eyedropper". I was referring to the "color picker". Sorry for the spam.

	Patrick Brosset <:pbro> Assignee
	Comment 7 • 8 years ago

This is true for all XUL documents, not only the browser toolbox, since XUL documents do not support injecting highlighters into them (see our highlighter documentation: https://wiki.mozilla.org/DevTools/Highlighter ).
So, typically, this means that the eyedropper doesn't work on pages like about:preferences too.

I unfortunately didn't think of this at all when working on bug 1262439, and this will be hard to fix (depends on platform bug 1094959).

At the very least, we should avoid throwing errors and perhaps even hide the icon when debugging XUL documents.

	Patrick Brosset <:pbro> Assignee
	Comment 8 • 8 years ago

I'm going to post a patch that disables the feature in non-HTML windows. This should prevent user confusion and will get rid of the error.

Now, to actually make this work in XUL windows, we need bug 1094959 which, as we're moving away from XUL for devtools, special Firefox pages and the browser itself, isn't likely to be high priority.

So I'm marking this as leave-open.

	Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]
	Comment 10 • 8 years ago
mozreview-review

Comment on attachment 8783519 [details]
Bug 1289543 - Disable the eyedropper on non-HTML documents;

https://reviewboard.mozilla.org/r/73308/#review71022

	Patrick Brosset <:pbro> Assignee
	Comment 12 • 8 years ago

Comment on attachment 8783519 [details]
Bug 1289543 - Disable the eyedropper on non-HTML documents;

Mike, some tests were failing with the last patch, because I didn't realize that the way the code was written, the eyedropper icon wouldn't appear for HTML docs either ... 
So I've updated it now, and the changes are sufficient to warrant another round of review I guess.

	Patrick Brosset <:pbro> Assignee
	Comment 13 • 8 years ago

Green try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6609a0b2add&group_state=expanded

	Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]
	Comment 15 • 8 years ago
mozreview-review

Comment on attachment 8783519 [details]
Bug 1289543 - Disable the eyedropper on non-HTML documents;

https://reviewboard.mozilla.org/r/73308/#review72174

	Pulsebot
	Comment 16 • 8 years ago

Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/f2b99a4da3cf
Disable the eyedropper on non-HTML documents; r=miker

	Ryan VanderMeulen [:RyanVM]
	Comment 17 • 8 years ago
bugherder

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

	Patrick Brosset <:pbro> Assignee
	Comment 18 • 8 years ago

Downgrading to P2 now that at least the eyedropper is disabled in non-HTML documents and knowing that the rest depends on platform bug 1094959 which has no priority at the moment, nor anyone working on (and it's unlikely that someone will knowing that XUL is eventually going away).

	BugBot [:suhaib / :marco/ :calixte]
	Comment 19 • 6 years ago

The leave-open keyword is there and there is no activity for 6 months.
:pbro, maybe it's time to close this bug?