Closed Bug 2021421 Opened 5 months ago Closed 4 months ago

Remove redundant lazy pref getters in SidebarPinsPromo

Categories

(Firefox :: Sidebar, task, P3)

task

Tracking

()

RESOLVED FIXED
151 Branch
Tracking Status
firefox151 --- fixed

People

(Reporter: sfoster, Assigned: sameekshadalvi7, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [fidefe-sidebar][lang=js][outreachy-sidebar-2026])

Attachments

(1 file)

In browser/components/sidebar/sidebar-pins-promo.mjs we define the SidebarPinsPromo class and in the constructor set up lazy preference getters so that this.verticalTabsEnabled will point to the value of the sidebar.verticalTabs pref, and this.dragToPinPromoDismissed points to the sidebar.verticalTabs.dragToPinPromo.dismissed pref. And change to either pref value will result in a call to this.requestUpdate() to re-render. That is all good. But in connectedCallback we erroneously register the exact same lazy pref getters again - these should be removed.

To help Mozilla out with this bug, here's the steps:

  1. Comment here on the bug that you want to volunteer to help.
    This will tell others that you're working on the next steps.
  2. Download and build the Firefox source code
  3. Start working on this bug. In this case it should be as simple as removing the redundant XPCOMUtils.defineLazyPreferenceGetter calls in connectedCallback.
    • If you have any problems with this bug,
      please comment on this bug and set the needinfo flag for the mentor (me) :sfoster.
      Also, you can find me and my teammates on the #sidebar channel on
      [Element/Matrix](https://chat.mozilla.org/#/room/#sidebar:mozilla.org
      most hours of most days.
  4. Any changes to fix this bug don't require re-compilation, so you should ensure you have artifact builds configured - it will be much faster. Build your change with ./mach build and test your change with
    ./mach test browser/components/sidebar/tests/browser/browser_sidebar_pinned_tabs.js.
    Also check your changes for adherence to our style guidelines by using ./mach lint
  5. Submit the patch for review.
    Mark me as a reviewer so I'll get an email to come look at your code.
  6. After a series of reviews and changes to your patch,
    I'll mark it for checkin or push it to autoland.
    Your code will soon be shipping to Firefox users worldwide!
  7. ...now you get to think about what kind of bug you'd like to work on next.
    Let me know what you're interested in and I can help you find your next contribution.
Whiteboard: [fidefe-sidebar] → [fidefe-sidebar] [lang=js]
Summary: Removed redundant lazy pref getters in SidebarPinsPromo → Remove redundant lazy pref getters in SidebarPinsPromo

Hey, I would like to work on this issue.

(In reply to Sameeksha from comment #1)

Hey, I would like to work on this issue.

Hi Sameeksha, go ahead using the instructions in the description above. When you submit a patch, it will get assigned to you, but in the meantime your note lets people know you are working on it.

Assignee: nobody → sameekshadalvi7
Status: NEW → ASSIGNED
Attachment #9555818 - Attachment description: WIP: Bug 2021421 - Remove redundant lazy preference getters in sidebar-pins-promo.mjs → Bug 2021421 - Remove redundant lazy preference getters in sidebar-pins-promo.mjs

Thank you Sam for giving me the opportunity to contribute! It was my first time working on such a large codebase and I learned a lot about the workflow using Phabricator and Bugzilla 🙌

Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 151 Branch
Whiteboard: [fidefe-sidebar] [lang=js] → [fidefe-sidebar][lang=js][outreachy-sidebar-2026]
QA Whiteboard: [qa-triage-done-c152/b151]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: