Closed Bug 1530199 Opened 9 months ago Closed 9 months ago

Remove hasSupportsHighlighters check in supportsEyeDropper

Categories

(DevTools :: Inspector, enhancement, P3)

enhancement

Tracking

(firefox67 fixed)

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: gl, Assigned: lloanalas, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

Currently, we have a supportsEyeDropper function in inspector.js that checks if the actor has "supportsHighlighter" method to handle backward support of different actors with potentially different methods. Since the actor method "supportsHighligther" was implemented in Bug 1306937 (back in 2016), we don't need this actor method check anymore since we are always executing await this.inspector.supportsHighlighters().

So, this bug is to clean up the try statement of supportsEyeDropper https://searchfox.org/mozilla-central/source/devtools/client/inspector/inspector.js#1036 by removing the hasSupportsHighlighters check and simplifying it to do a single return:

return await this.inspector.supportsHighlighters()

Hi. I am an outreachy applicant and would like to work on this. May you please assign it to me? Thanks :)

(In reply to jk025523 from comment #1)

Hi. I am an outreachy applicant and would like to work on this. May you please assign it to me? Thanks :)

Hi, feel free to submit a patch.

Hi, I'm an outreachy applicant. This bug is assigned, if no one has picked it up yet, can I be assigned to it? (in case previous requester was assigned another bug). I was reading the How to Submit a Patch instructions and noticed you have to be assigned to a bug to submit a patch.

Thanks a ton!

(In reply to lloanalas from comment #3)

Hi, I'm an outreachy applicant. This bug is assigned, if no one has picked it up yet, can I be assigned to it? (in case previous requester was assigned another bug). I was reading the How to Submit a Patch instructions and noticed you have to be assigned to a bug to submit a patch.

Thanks a ton!

Not assigned*

Assignee: nobody → lloanalas
Status: NEW → ASSIGNED

Hi, I was not assigned another bug, and when I requested to be assigned this bug, the project maintainer @Gabriel refused to. Why a different decision this time @Gabriel? I was preparing to submit a patch for this bug.

(In reply to jk025523 from comment #5)

Hi, I was not assigned another bug, and when I requested to be assigned this bug, the project maintainer @Gabriel refused to. Why a different decision this time @Gabriel? I was preparing to submit a patch for this bug.

Oh my mistake, I actually only read the comments in my email and didn't see the previous comments.

Assignee: lloanalas → jk025523

(In reply to Gabriel [:gl] (ΦωΦ) from comment #6)

(In reply to jk025523 from comment #5)

Hi, I was not assigned another bug, and when I requested to be assigned this bug, the project maintainer @Gabriel refused to. Why a different decision this time @Gabriel? I was preparing to submit a patch for this bug.

Oh my mistake, I actually only read the comments in my email and didn't see the previous comments.

I understand. I had started working on it as well. No worries. I was just reading on how to submit a patch as mine is ready.

Thanks regardless :)

Cleans up the try statement within supportsEyeDropper function.

(In reply to lloanalas from comment #8)

Created attachment 9046834 [details]
cleans up the try statement in supportsEyeDropper - bug 1530199

Cleans up the try statement within supportsEyeDropper function.

Please feel free to disregard. Since I had already done the work, I figured I could use this to figure out how to do an actual commit - to get a hang of the process.

Attachment #9046834 - Attachment description: cleans up the try statement in supportsEyeDropper - bug 1530199 → Bug 1530199 - Cleans up the try statement in supportsEyeDropper. r=gl
Assignee: jk025523 → lloanalas
Attachment #9046834 - Attachment description: Bug 1530199 - Cleans up the try statement in supportsEyeDropper. r=gl → Bug 1530199 - Remove hasSupportsHighlighters check in supportsEyeDropper r=gl
Pushed by gluong@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9b1e57540da7
Remove hasSupportsHighlighters check in supportsEyeDropper r=gl
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.