2060396 - Remove OSToolbarButtonPressed and related code.

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, task)

Comment 1

[Emilio Cobos Álvarez [:emilio]]

I did the digging, and this seems to come from bug 363415, and then morphed shape in multiple ways, but mostly through refactors like bug 743975.

But I don't think macOS titlebars have a "toolbar" button anymore.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2060396 - Remove OSToolbarButtonPressed and related code. r=#mac-reviewers

I did the digging, and this seems to come from bug 363415, and then
morphed shape in multiple ways, but mostly through refactors like bug
743975.

But I don't think macOS titlebars have a "toolbar" button anymore. And
I'm pretty sure even if we reached this code this wouldn't work as
expected, since nsIWebBrowserChrome::SetChromeFlags implementations
don't do much (clean up for that incoming).

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2060396 - Remove toggletoolbar attribute support. r=#mac-reviewers

I did more digging and this seems to be basically dead code. It seems
older versions of OSX had some sort of toolbar control button? But it's
documented to have no effect at all nowadays:

https://developer.apple.com/documentation/appkit/nswindow/showstoolbarbutton

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2060396 - Make chrome flags immutable. r=vhilla,smaug

We only deal with a couple of these flags anyways. The bar changes could
theoretically work, but we don't use them (other than in a test).

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2060396 - Remove CHROME_LOCATIONBAR / chromeclass-location. r=#urlbar-reviewers!

Nothing can set this now. Even popup windows opened with location=no
show the urlbar anyway, and we have tests for it.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/06e2e4f07d85
https://hg.mozilla.org/integration/autoland/rev/d8d2efb042bd
Remove OSToolbarButtonPressed and related code. r=mac-reviewers,mstange

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f9a9af7cd0d2
https://hg.mozilla.org/integration/autoland/rev/2eab102c0d6b
Remove toggletoolbar attribute support. r=places-reviewers,mak

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2060396 - Throw on BarProp.visible setter. r=vhilla

As per discussion in D316217. There's a sessionstore caller, but
sessionstore will open the window with the right window features
regardless, so it's effectively a no-op (and after D316217 it is
definitely a no-op).

I tested restoring sessions with popup windows open and such just in
case it wasn't covered by tests, and it works alright.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/71e2267c243c
https://hg.mozilla.org/integration/autoland/rev/d89dc224aea3
Make chrome flags immutable. r=vhilla

Comment 10

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/d8d2efb042bd
https://hg.mozilla.org/mozilla-central/rev/2eab102c0d6b
https://hg.mozilla.org/mozilla-central/rev/d89dc224aea3

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Let me move the unlanded work to a new bug for better tracking.

Comment 12

[Phabricator Automation]

Comment on attachment 9621553 [details]
Bug 2060396 - Remove CHROME_LOCATIONBAR / chromeclass-location. r=#urlbar-reviewers!

Revision D316218 was moved to bug 2064683. Setting attachment 9621553 [details] to obsolete.

Comment 13

[Phabricator Automation]

Comment on attachment 9625528 [details]
Bug 2060396 - Throw on BarProp.visible setter. r=vhilla

Revision D317990 was moved to bug 2064683. Setting attachment 9625528 [details] to obsolete.