Closed Bug 1446316 Opened 2 years ago Closed 2 years ago

Proposal to remove the new-but-unused color picker code

Categories

(DevTools :: Inspector, enhancement)

enhancement
Not set

Tracking

(firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: pbro, Assigned: pbro)

References

Details

Attachments

(1 file)

Some time ago, we started working on a new color picker that had more functionalities, like contrast ratio. 
This was unfortunately never finished, and the code is unused now. It is behind a pref.

In fact, ever since bug 1433929, turning this pref ON breaks the inspector completely. So it's more than unused, it's broken. And we didn't have any tests in place to tell us that it was broken (it has been the case for 2 weeks, and someone noticed 3 days ago in bug 1445171).

I'm proposing to remove this code. If we ever find the time to pursue this effort again, we can easily salvage the code from either bugzilla or mercurial history anyway.

Keeping it around has drawbacks:
- it sometimes breaks without us knowing, confusing people and wasting investigation time,
- if we do notice when it breaks, we're forced to fix it for no user value since it's OFF by default,
- it makes the code more complex and harder to navigate.
There we go, here is a patch to remove the code. I *think* it covers everything, I hope I didn't miss any spot.

Oh, I guess the other reason for removing rather than keeping around is: we don't have this in our immediate roadmap of things to work on now. If we did, it would be better to just keep the code. But right now, I can't guaranty that this code won't stay for another few months or years still unused and broken. So, better remove it all.

Also, if you just hg backout this commit, you get the code again! So, no hard feelings, it's still there if we want it.
See Also: → 1445171
Putting this back on Gabriel's radar.
Gabriel: can we proceed with this?
Flags: needinfo?(gl)
Comment on attachment 8959493 [details]
Bug 1446316 - Removing the unused and broken new colorPicker widget;

https://reviewboard.mozilla.org/r/228284/#review239816
Attachment #8959493 - Flags: review?(gl) → review+
Comment on attachment 8959493 [details]
Bug 1446316 - Removing the unused and broken new colorPicker widget;

https://reviewboard.mozilla.org/r/228284/#review239818

::: commit-message-fcb11:1
(Diff revision 1)
> +Bug 1446316 - Removing the unused and broken new colorPicker widge; r=gl

typo on widget
Flags: needinfo?(gl)
Thanks Gabriel. I addressed the typo, and pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd539e1bfd8f53a732518ccab1a48309e39b7ab6
Going to land this now.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0888bc574df1
Removing the unused and broken new colorPicker widget; r=gl
https://hg.mozilla.org/mozilla-central/rev/0888bc574df1
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.