1438511 - gUIDensity.update is causing initialization of the sidebar browser at startup

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[(inactive) Nihanth Subramanya [:nhnt11]]

From my investigation, this is happening even before SidebarUI.init() which is bad. Patch coming soon.

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1438511 - Don't update sidebar UI density before SidebarUI is initialized, or if the sidebar is not open.

Review commit: https://reviewboard.mozilla.org/r/220512/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220512/

Comment 2

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1438511 - Copy top window's uidensity attribute in sidebarUtils.js instead of forcing gUIDensity.update().

Review commit: https://reviewboard.mozilla.org/r/220514/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220514/

Comment 3

[Florian Quèze [:florian]]

Comment on attachment 8951247 [details]
Bug 1438511 - Don't update sidebar UI density before SidebarUI is initialized, or if the sidebar is not open.

https://reviewboard.mozilla.org/r/220512/#review226486

Works well, thanks!

::: browser/base/content/browser-sidebar.js:51
(Diff revision 1)
>    _icon: null,
>    _reversePositionButton: null,
>    _switcherPanel: null,
>    _switcherTarget: null,
>    _switcherArrow: null,
> +  _inited: null,

This should just be initialized to false

::: browser/base/content/browser-sidebar.js:54
(Diff revision 1)
>    _switcherTarget: null,
>    _switcherArrow: null,
> +  _inited: null,
> +
> +  get initialized() {
> +    return !!this._inited;

and then you don't need the !! here.

::: browser/base/content/browser.js:5639
(Diff revision 1)
>    update(mode) {
>      if (mode == null) {
>        mode = this.getCurrentDensity().mode;
>      }
>  
> -    let docs = [
> +    let docs = [ document.documentElement ];

nit: there's usually no space between '[', ']' and the array content when the initialization is on a single line.

Comment 4

[Florian Quèze [:florian]]

Comment on attachment 8951250 [details]
Bug 1438511 - Copy top window's uidensity attribute in sidebarUtils.js instead of forcing gUIDensity.update().

https://reviewboard.mozilla.org/r/220514/#review226488

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8951247 [details]
Bug 1438511 - Don't update sidebar UI density before SidebarUI is initialized, or if the sidebar is not open.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220512/diff/1-2/

Comment 6

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8951250 [details]
Bug 1438511 - Copy top window's uidensity attribute in sidebarUtils.js instead of forcing gUIDensity.update().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220514/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be58311261fc
Don't update sidebar UI density before SidebarUI is initialized, or if the sidebar is not open. r=florian
https://hg.mozilla.org/integration/autoland/rev/3ae4195fba97
Copy top window's uidensity attribute in sidebarUtils.js instead of forcing gUIDensity.update(). r=florian

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/be58311261fc
https://hg.mozilla.org/mozilla-central/rev/3ae4195fba97