Closed Bug 1937191 Opened 2 months ago Closed 1 month ago

Removed sidebar-button reappears on browser restart

Categories

(Firefox :: Sidebar, defect, P2)

defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: nsharpley, Unassigned)

References

Details

(Whiteboard: [fidefe-sidebar])

STR:

  1. Set sidebar-revamp to true.
  2. Remove the sidebar-button from the nav bar in Customize Mode.
  3. Set sidebar-revamp to false.
  4. Set sidebar-revamp to true.

Expected:
The sidebar button should no longer be in the nav bar

Actual:
The sidebar button has been added back to the nav bar.

The button is added back in here: https://searchfox.org/mozilla-central/rev/d8f4a94563b0a28c485f0dbfaf56b92434242d1e/browser/components/customizableui/CustomizableUI.sys.mjs#221-227

We should rethink how we want to handle if a user has actually removed the button themselves versus a user switching on sidebar.revamp for the first time.

We need to differentiate between the case where the button has been explicitly removed by the user, vs. it's not there because they are just switching to sidebar.revamp (at which point we currently insert it for them)

Priority: -- → P2
Whiteboard: [fidefe-sidebar]
Severity: -- → S3

I see this error when the browser starts with sidebar.revamp set to true

TypeError: can't access property "dataset", this.toolbarButton is null
    updateToolbarButton chrome://browser/content/sidebar/browser-sidebar.js:1097
    onCreated resource:///modules/CustomizableWidgets.sys.mjs:300
    aEventName resource:///modules/CustomizableUI.sys.mjs:3475
    buildWidget resource:///modules/CustomizableUI.sys.mjs:2185
    getWidgetNode resource:///modules/CustomizableUI.sys.mjs:1480
    buildArea resource:///modules/CustomizableUI.sys.mjs:1274
    registerToolbarNode resource:///modules/CustomizableUI.sys.mjs:1185
    registerToolbarNode resource:///modules/CustomizableUI.sys.mjs:4624
    onDOMContentLoaded chrome://browser/content/browser-init.js:169
CustomizableUI.sys.mjs:3480:17

maybe it is related, maybe a regression from bug 1897411

(In reply to tabmix.onemen from comment #2)

I see this error when the browser starts with sidebar.revamp set to true

TypeError: can't access property "dataset", this.toolbarButton is null
    updateToolbarButton chrome://browser/content/sidebar/browser-sidebar.js:1097
    onCreated resource:///modules/CustomizableWidgets.sys.mjs:300
    aEventName resource:///modules/CustomizableUI.sys.mjs:3475
    buildWidget resource:///modules/CustomizableUI.sys.mjs:2185
    getWidgetNode resource:///modules/CustomizableUI.sys.mjs:1480
    buildArea resource:///modules/CustomizableUI.sys.mjs:1274
    registerToolbarNode resource:///modules/CustomizableUI.sys.mjs:1185
    registerToolbarNode resource:///modules/CustomizableUI.sys.mjs:4624
    onDOMContentLoaded chrome://browser/content/browser-init.js:169
CustomizableUI.sys.mjs:3480:17

maybe it is related, maybe a regression from bug 1897411

It's unrelated but was fixed in bug 1939639.

Based on the updated STR, I don't think this is a bug - moving the toolbar button when the sidebar.revamp pref is flipped to true was by design (and any further movement by the user after that is respected). We don't have the concept of flipping the pref the first time, nor do I think its needed at this point since this pref will eventually be removed.

Nikki and Sam, if either of you thinks there's some other issue at play here please chime in and/or reopen the bug.

Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.