The new eyedropper does not work in XUL documents

ASSIGNED
Assigned to

Status

()

Firefox
Developer Tools: Inspector
P2
normal
ASSIGNED
2 years ago
2 years ago

People

(Reporter: magicp, Assigned: pbro)

Tracking

(Depends on: 1 bug, {leave-open})

Trunk
leave-open
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 affected, firefox51 affected)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 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.
(Reporter)

Comment 1

2 years ago
Regression range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=e0bc88708ffed39aaab1fbc0ac461d93561195de&tochange=a7a882d122e36defe6c2a102a28ae7dfc16311c5
Blocks: 1262439
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Developer Tools: Inspector
OS: Unspecified → All
Hardware: Unspecified → All
(Reporter)

Updated

2 years ago
status-firefox50: --- → affected
status-firefox51: --- → affected
Recent regression from eyedropper migration. Eyedropper cannot be used in Browser Toolbox -> P1

Inspector bug triage, filter on CLIMBING SHOES.
Priority: -- → P1
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)?
(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 ?
Flags: needinfo?(albert)
(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 :)
Flags: needinfo?(albert)
... 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.
(Assignee)

Comment 7

2 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.
Depends on: 1094959
Summary: The new eyedropper does not work in the Browser Toolbox → The new eyedropper does not work in XUL documents
(Assignee)

Comment 8

2 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.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Keywords: leave-open
Comment hidden (mozreview-request)
Comment on attachment 8783519 [details]
Bug 1289543 - Disable the eyedropper on non-HTML documents;

https://reviewboard.mozilla.org/r/73308/#review71022
Attachment #8783519 - Flags: review?(mratcliffe) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 12

2 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.
Attachment #8783519 - Flags: review+ → review?(mratcliffe)
Comment hidden (obsolete)
Comment on attachment 8783519 [details]
Bug 1289543 - Disable the eyedropper on non-HTML documents;

https://reviewboard.mozilla.org/r/73308/#review72174
Attachment #8783519 - Flags: review?(mratcliffe) → review+

Comment 16

2 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
(Assignee)

Comment 18

2 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).
Priority: P1 → P2
(Reporter)

Updated

2 years ago
Depends on: 1306937
You need to log in before you can comment on or make changes to this bug.