1563349 - Support NSPopoverTouchBarItem and NSScrollView in the Touch Bar

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, enhancement)

Description

[Harry Twyford]

The popover will hold the scroll view. This allows us to create new Touch Bar inputs that behave like our Share button does now.

Comment 1

[Harry Twyford]

Attachment: Bug 1563349 - Support NSPopoverTouchBarItem and NSScrollView in the Touch Bar. r?spohl,mikedeboer

Comment 2

[Harry Twyford]

Here's an initial patch. There are two outstanding items:

1. Completing bug 1563351 (there's a bit of placeholder code in https://phabricator.services.mozilla.com/D38563 in the meantime).
2. Resolving a shutdown crash: https://paste.mozilla.org/A6RrcBrE. (It looks related to Localization?)

I don't expect bug 1563351 will take very long and I expect the fix for (2) won't require a lot of changed code relative to the size of the two patches, so I thought I'd post this now to get the ball rolling on review.

Comment 3

[Harry Twyford]

Attachment: Bug 1563349 - Part 1 - JS changes to support TouchBarInput children. r?mikedeboer

Depends on D34926

Comment 4

[Harry Twyford]

Attachment: Bug 1563349 - Part 2 - Support TouchBarInput children. r?spohl

Depends on D47618

Comment 5

[Harry Twyford]

Attachment: Bug 1563349 - Part 3 - Update TouchBarInputs in-place. r?spohl

Depends on D47619

Comment 6

[Harry Twyford]

Attachment: Bug 1563349 - Part 4 - Support NSPopoverTouchBarItem. r?spohl

Depends on D47620

Comment 7

[Harry Twyford]

Attachment: Bug 1563349 - Part 5 - Support NSScrollView in the Touch Bar. r?spohl

Depends on D47621

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dcbddc5738c3
Part 1 - JS changes to support TouchBarInput children. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/db50ea44f0b1
Part 2 - Support TouchBarInput children. r=spohl
https://hg.mozilla.org/integration/autoland/rev/cff2050ff540
Part 3 - Update TouchBarInputs in-place. r=spohl
https://hg.mozilla.org/integration/autoland/rev/81b93962243b
Part 4 - Support NSPopoverTouchBarItem. r=spohl
https://hg.mozilla.org/integration/autoland/rev/4b4a71e170bd
Part 5 - Support NSScrollView in the Touch Bar. r=spohl

Comment 9

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/dcbddc5738c3
https://hg.mozilla.org/mozilla-central/rev/db50ea44f0b1
https://hg.mozilla.org/mozilla-central/rev/cff2050ff540
https://hg.mozilla.org/mozilla-central/rev/81b93962243b
https://hg.mozilla.org/mozilla-central/rev/4b4a71e170bd

Comment 10

[Stefan Hindli [:stefan_hindli]]

Backed out 8 changesets (Bug 1563350, Bug 1563351, Bug 1563349, Bug 1567939) on suspicion of causing OS X crashes https://bugzilla.mozilla.org/show_bug.cgi?id=1587351 CLOSED TREE a=backout

Comment 11

[Dave Townsend [:mossop]]

I was seeing build failures locally with this with an error at https://hg.mozilla.org/mozilla-central/annotate/355b0329bd95d2a9573c04ece889c734da00f080/widget/cocoa/nsTouchBar.mm#l227 complaining about assigning to a readonly property (customizationLabel does appear to be readonly: https://developer.apple.com/documentation/appkit/nstouchbaritem/2544693-customizationlabel).

Comment 12

[Harry Twyford]

Good to know. We were previously setting customizationLabel on a subclass of NSTouchBarItem, where it's readwrite: https://developer.apple.com/documentation/appkit/nscustomtouchbaritem/2544770-customizationlabel?language=objc.

It builds fine on my machine (presumably it casts item properly before setting item.customizationLabel) and this doesn't seem related to the crashes reported in bug 1587351, but I'll fix this nonetheless before attempting to re-land.

Comment 13

[Harry Twyford]

I believe I've fixed the crash in bug 1587351. Here's a green try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d2d5c93d816461ac34c0572cdb9a98b1d06782f. I've also run the fixed build by people who encountered the previous crash and they can no longer replicate the crash. I'm going to re-land this.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3c851b5af54
Part 1 - JS changes to support TouchBarInput children. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/4f2f54e2c7ab
Part 2 - Support TouchBarInput children. r=spohl
https://hg.mozilla.org/integration/autoland/rev/c4002fd5b15d
Part 3 - Update TouchBarInputs in-place. r=spohl
https://hg.mozilla.org/integration/autoland/rev/eb6c081d3ec0
Part 4 - Support NSPopoverTouchBarItem. r=spohl
https://hg.mozilla.org/integration/autoland/rev/407be8a26112
Part 5 - Support NSScrollView in the Touch Bar. r=spohl

Comment 15

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/f3c851b5af54
https://hg.mozilla.org/mozilla-central/rev/4f2f54e2c7ab
https://hg.mozilla.org/mozilla-central/rev/c4002fd5b15d
https://hg.mozilla.org/mozilla-central/rev/eb6c081d3ec0
https://hg.mozilla.org/mozilla-central/rev/407be8a26112