Closed
Bug 1295008
Opened 9 years ago
Closed 8 years ago
Don't run duplicate the eyedropper
Categories
(DevTools :: Inspector, defect, P2)
DevTools
Inspector
Tracking
(firefox50 fixed, firefox51 verified)
VERIFIED
FIXED
Firefox 51
People
(Reporter: magicp.jp, Assigned: pbro)
References
Details
Attachments
(2 files)
648.05 KB,
video/mp4
|
Details | |
58 bytes,
text/x-review-board-request
|
miker
:
review+
ritu
:
approval-mozilla-aurora+
|
Details |
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160813030202
Steps to reproduce:
1. Start Nightly
2. Run the eyedropper from menubar or PanelUI-contents
3. Open DevTools (Ctrl+Shift+I)
4. Check the eyedropper color (The checked status is not reflected)
5. Click the eyedropper icon
6. Check run duplicate the eyedropper
Actual results:
The eyedropper runs duplicate. And the checked status is not reflected if running it from menubar or PanelUI-contents.
Expected results:
Don't run duplicate the eyedropper and the checked status is reflected.
Has STR: --- → yes
status-firefox50:
--- → affected
status-firefox51:
--- → affected
Component: Untriaged → Developer Tools: Inspector
OS: Unspecified → All
Hardware: Unspecified → All
Assignee | ||
Updated•8 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8783479 [details]
Bug 1295008 - Hide the previous eyedropper when we request to show a new one;
https://reviewboard.mozilla.org/r/73260/#review71646
Attachment #8783479 -
Flags: review?(mratcliffe) → review+
Assignee | ||
Comment 7•8 years ago
|
||
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/9e3157084c5b
Hide the previous eyedropper when we request to show a new one; r=miker
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8783479 [details]
Bug 1295008 - Hide the previous eyedropper when we request to show a new one;
Approval Request Comment
[Feature/regressing bug #]: bug 1262439
[User impact if declined]: The eyedropper can be open from a few different locations in devtools: the command line, the inspector, the color picker, the devtools browser menu. Opening the eyedropper from the inspector, and then from the command line for example, will end up displaying 2 eyedroppers in the page if we decline this uplift request.
[Describe test coverage new/current, TreeHerder]: All current eyedropper tests still pass. No new test was added. This was only manually tested.
[Risks and why]: This is a devtools JS-only change. The risk is rather low. The code changes are located in the eyedropper itself, so if there were new bugs introduced there, only the devtools eyedropper would suffer from them. There are also a few code changes in the inspector which, in itself, could be a bit more risky, but these changes are only in the functions that show the eyedropper, so the inspector itself would still load and function properly even if these changes were wrong.
In any case, there are no impacts to the browser UI itself.
[String/UUID change made/needed]: None
Attachment #8783479 -
Flags: approval-mozilla-aurora?
Comment 10•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Hello magicp, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(magicp.jp)
Reporter | ||
Comment 12•8 years ago
|
||
Hello Ritu, I have confirmed this in 51 (Build ID: 20160826030226). Run duplicate was fixed, but the checked status is not reflected.
Flags: needinfo?(magicp.jp)
Hey Patrick, based on the verification in comment 12, we have only partially fixed the problem. Are you able to repro this on your end as well? I can wait for a patch-2 and we can uplift both to Aurora50 next week.
Flags: needinfo?(pbrosset)
Assignee | ||
Comment 14•8 years ago
|
||
(In reply to magicp from comment #12)
> Hello Ritu, I have confirmed this in 51 (Build ID: 20160826030226). Run
> duplicate was fixed, but the checked status is not reflected.
Right, I missed this in the original STR in comment 0.
(In reply to Ritu Kothari (:ritu) from comment #13)
> Hey Patrick, based on the verification in comment 12, we have only partially
> fixed the problem. Are you able to repro this on your end as well? I can
> wait for a patch-2 and we can uplift both to Aurora50 next week.
Yes I can repro, I missed this detail when working on the fix.
I prefer not to add a second patch here. I'll file a separate bug for the remaining issue, and will ask uplift there if needed.
Flags: needinfo?(pbrosset)
(In reply to Patrick Brosset <:pbro> from comment #14)
> I prefer not to add a second patch here. I'll file a separate bug for the
> remaining issue, and will ask uplift there if needed.
Ok. I am fine with that. You or magicp can file another bug to track that. I'll approve this one for uplift then.
Flags: needinfo?(magicp.jp)
Status: RESOLVED → VERIFIED
Comment on attachment 8783479 [details]
Bug 1295008 - Hide the previous eyedropper when we request to show a new one;
Fix was verified on Nightly, Aurora50+
Attachment #8783479 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 17•8 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 18•8 years ago
|
||
(In reply to Ritu Kothari (:ritu) from comment #15)
> Ok. I am fine with that. You or magicp can file another bug to track that.
> I'll approve this one for uplift then.
Hello Patrick, Thanks for filing Bug 1298769.
Flags: needinfo?(magicp.jp)
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•