1914359 - The Vertical Sidebar and Tabs are not displayed when enabled if a sidebar is already opened

Status: VERIFIED FIXED

(Firefox :: Sidebar, defect, P1)

Description

[Rares Doghi, Desktop QA]

Attachment: Sidebar.mp4

Found in

• 131.0a1 (2024-08-21)

Affected versions

• 131.0a1 (2024-08-21)

Affected platforms

• all

Steps to reproduce

1. Enable ChatGPT from Firefox Labs
2. Check the Sidebar and Vertical Tabs buttons from the Firefox Labs page
3. Click the Sidebar button added to the Toolbar

Expected result

• The Sidebar and Vertical tabs should enable.

Actual result

• The Sidebar and Vertical tabs are not displayed.
  Clicking the Sidebar button from the toolbar does not work.

Disabling and Enabling the Sidebar again will show the Actual Sidebar and vertical tabs without issues.

Regression range
Not a regression.

Comment 1

[Sarah Clements [:sclements]]

This is interesting... I have not been able to reproduce this. Are you still seeing it?

Comment 2

[Ed Lee :Mardak]

very strange… at 0:34, unchecking chatbot does finally close the sidebar (but not from previous attempts), and it's checking SidebarController to hide. If you can reproduce the issue, checking the browser console for errors would help

Comment 3

[Rares Doghi, Desktop QA]

Attachment: AiSiodebars.mp4

I can still reproduce this issue in our latest Nightly build, only something a lot worse happens now. Please see the video.

Comment 4

[Rares Doghi, Desktop QA]

This is the only error in Console:
Uncaught (in promise) TypeError: can't access property "expanded", this._sidebarMain is undefined
toggleExpanded chrome://browser/content/sidebar/browser-sidebar.js:1013
hide chrome://browser/content/sidebar/browser-sidebar.js:1432
toggleRevampSidebar chrome://browser/content/sidebar/browser-sidebar.js:566

Comment 5

[Sarah Clements [:sclements]]

It looks like this was fixed in the latest Nightly 131.0a1, thanks to Kelly's work in bug 1913157. Rares, can you confirm it also looks like the issue is fixed for you?

I manually tested this in the latest 130 beta and I do see the same issue if you manually flip the sidebar.revamp pref flipped after enabling the sidebar chatbot in labs (but its otherwise not affected by this bug). Its too late for a beta uplift, and its perhaps a question whether we'd want this fix for a dot release, just knowing that the prefs are out in the wild even though new sidebar isn't officially in labs for 130. Gijs, mardak, any thoughts on this?

Comment 6

[:Gijs (out for now; he/him)]

(In reply to Sarah Clements [:sclements] from comment #5)

I manually tested this in the latest 130 beta and I do see the same issue if you manually flip the sidebar.revamp pref flipped after enabling the sidebar chatbot in labs (but its otherwise not affected by this bug).

Which issue - the one in comment 0 or the hang shown in Rares' most recent video?

Its too late for a beta uplift, and its perhaps a question whether we'd want this fix for a dot release, just knowing that the prefs are out in the wild even though new sidebar isn't officially in labs for 130. Gijs, mardak, any thoughts on this?

Does a restart fix this?

Comment 7

[Sarah Clements [:sclements]]

(In reply to :Gijs (he/him) from comment #6)

(In reply to Sarah Clements [:sclements] from comment #5)

I manually tested this in the latest 130 beta and I do see the same issue if you manually flip the sidebar.revamp pref flipped after enabling the sidebar chatbot in labs (but its otherwise not affected by this bug).

Which issue - the one in comment 0 or the hang shown in Rares' most recent video?

Both issues.

Its too late for a beta uplift, and its perhaps a question whether we'd want this fix for a dot release, just knowing that the prefs are out in the wild even though new sidebar isn't officially in labs for 130. Gijs, mardak, any thoughts on this?

Does a restart fix this?

Yes, it does. And to be clear, its only the sidebar toolbar button not working in 130 beta after sidebar.revamp is flipped to true, not the strange split screen from the second video.

Comment 8

[Rares Doghi, Desktop QA]

This issue still occurs in our latest Nightly build: 20240827213859 , it just has different behavior now (latest screen recording), before It didn't do anything, but now it duplicates the content for some reason.
And yes if we Disable and Enable the Sidebar again from the Firefox Labs it will return to normal and show the actual sidebar but this only happens if we disable and re-enable the Sidebar button from Firefox Labs (about:preferences#experimental)

In Beta we dont have the Sidebar and Vertical tabs checkboxes in Firefox Labs.

Steps to reproduce:

1. Enable ChatGPT from Firefox Labs
2. Check the Sidebar and Vertical Tabs buttons from the Firefox Labs page
3. Click the Sidebar button added to the Toolbar

Comment 9

[:Gijs (out for now; he/him)]

(In reply to Rares Doghi, Desktop QA from comment #8)

In Beta we dont have the Sidebar and Vertical tabs checkboxes in Firefox Labs.

Sure, but you can set the same prefs from about:config (sidebar.revamp and sidebar.verticalTabs).

As long as we don't hang the browser completely (which is sort of what the second screencast looks like) I think we don't need to worry about this for 130. On macOS at least, I don't see a hang. If I follow these steps on a clean profile in Nightly:

Steps to reproduce:

1. Enable ChatGPT from Firefox Labs
2. Check the Sidebar and Vertical Tabs buttons from the Firefox Labs page
3. Click the Sidebar button added to the Toolbar

Then the sidebar closes and immediately reopens when clicking the sidebar button.

Comment 10

[Rares Doghi, Desktop QA]

If we simply set the sidebar.revamp and sidebar.verticalTabs in Beta from about:config it will not add the sidebar button on the toolbar, after a restart everything works great, so I cant reproduce the issue in 130.

In Nightly if I enable Chat GPT first, and then Click the Sidebar checkbox button and Vertical Tabs from the Firefox Labs, the Sidebar button is displayed on the Toolbar and cause the above issues.

Comment 11

[Sarah Clements [:sclements]]

(In reply to Rares Doghi, Desktop QA from comment #10)

If we simply set the sidebar.revamp and sidebar.verticalTabs in Beta from about:config it will not add the sidebar button on the toolbar, after a restart everything works great, so I cant reproduce the issue in 130.

Yes, but if you drag the sidebar button into the nav bar in order to click the button that's where I see the issue in beta. I don't see a hang, just that the button doesn't work to toggle the sidebar with the prefs flipped until after a restart.

Comment 12

[Sarah Clements [:sclements]]

Rares, sorry where are we at with this? Are you still seeing an issue in Nightly?

Comment 13

[Rares Doghi, Desktop QA]

Yes this issue still occurs in our latest Nightly build 132.0a1 (2024-09-04). @Sarah

Comment 14

[Ed Lee :Mardak]

(In reply to Rares Doghi, Desktop QA from comment #4)

Uncaught (in promise) TypeError: can't access property "expanded", this._sidebarMain is undefined
toggleExpanded chrome://browser/content/sidebar/browser-sidebar.js:1013
hide chrome://browser/content/sidebar/browser-sidebar.js:1432
toggleRevampSidebar chrome://browser/content/sidebar/browser-sidebar.js:566

The entrypoint is the sidebar.revamp pref change handler from bug 1906140, and it's newly calling into toggleExpanded from bug 1899580. It's not specific to chatbot -- having any sidebar open goes through this code path when enabling sidebar (or generally flipping sidebar.revamp).

https://searchfox.org/mozilla-central/rev/cc01f11adfacca9cd44a75fd140d2fdd8f9a48d4/browser/components/sidebar/browser-sidebar.js#1021

Previously hide() had this.sidebarMain.expanded = … but now it calls toggleExpanded() doing this._sidebarMain.expanded = …

Basically, the sidebarMain getter wasn't accessed before to initialize _sidebarMain

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1899580

:sfoster, since you are the author of the regressor, bug 1899580, could you take a look?

For more information, please visit BugBot documentation.

Comment 16

[Sam Foster [:sfoster] (he/him)]

Thanks for tracking that down :marak, I can take this.

Comment 17

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1914359 - Ensure new sidebar launcher shows when toggling the pref while a legacy sidebar is open. r?#sidebar-reviewers

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b5b080f35d3
Ensure new sidebar launcher shows when toggling the pref while a legacy sidebar is open. r=sidebar-reviewers,Gijs

Comment 19

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/7b5b080f35d3

Comment 20

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:sfoster, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox132 to wontfix.

For more information, please visit BugBot documentation.

Comment 21

[Rares Doghi, Desktop QA]

Verified as fixed in our latest Nightly 133.0a1 (2024-10-04).

Comment 22

[Sam Foster [:sfoster] (he/him)]

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #20)

The patch landed in nightly and beta is affected.
:sfoster, is this bug important enough to require an uplift?

No I think this is intentionally wontfix for 132.

Comment 23

[Rares Doghi, Desktop QA]

Updating the main status flag.