Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash displaying shields in CarPlay #1665

Merged
merged 2 commits into from
Sep 12, 2018
Merged

Fix crash displaying shields in CarPlay #1665

merged 2 commits into from
Sep 12, 2018

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Sep 9, 2018

Before adding an attributed string to a maneuver’s attributed instructions, downgrade any instance of a custom text attachment subclass, such as ShieldAttachment, to NSTextAttachment. This ensures that the attributed string can be marshaled over to the CarPlay unit. Unfortunately, even standard NSTextAttachments don’t show up in the CarPlay simulator, but apparently they do show up on an actual CarPlay device.

Along the way, secondary and tertiary text instructions are now included in plain-text maneuver instructions for CarPlay units that lack support for attributed strings.

Fixes #1652.

/cc @JThramer

@1ec5 1ec5 added - crash CarPlay Bugs, improvements and feature requests on Apple CarPlay labels Sep 9, 2018
@1ec5 1ec5 self-assigned this Sep 9, 2018
@1ec5 1ec5 requested a review from JThramer September 9, 2018 07:22
@1ec5 1ec5 force-pushed the 1ec5-carplay-shields branch 2 times, most recently from 7f6f53e to 00a0461 Compare September 9, 2018 07:30
extension NSMutableAttributedString {
func canonicalizeAttachments() {
enumerateAttribute(.attachment, in: NSRange(location: 0, length: length), options: []) { (value, range, stop) in
guard let attachment = value as? NSTextAttachment, type(of: attachment) != NSTextAttachment.self else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@1ec5 this guard... it doesn't read right!!! Would this be why some of the shields are not showing up sometimes?

Copy link
Contributor

@JThramer JThramer Sep 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vincethecoder To me, it's reading as "The attachment is a subclass of NSAttachment, but not the actual class NSAttachment.self"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JThramer Yes, I'm reading this as we are looking for an attachment which is a subclass of NSTextAttachment but isn't of type NSTextAttachment. But I suppose that's why we make the attempt to canonicalize the attachment(s). Btw, is it possible with our attachments?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vincethecoder yeah, this is likely because of limitations of the NSAttributedString logic in CarPlay.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The application marshals the attributed string over to the CarPlay simulator or device for rendering. At least for the simulator if not also for the device, the attributed string is marshaled over XPC. It isn’t possible to marshal over instances of our custom NSTextAttachment subclasses, especially not the executable code in these subclasses. “Canonicalizing” the attributed string consists of replacing each subclassed attachment with a plain-vanilla NSTextAttachment that is sure to marshal over correctly.

There are other limitations to attributed strings in CarPlay maneuver instructions, such as being unable to change fonts or font sizes. However, those limitations aren’t limited to CarPlay. For example, on macOS, you can use attributed strings in title bars, but many attributes are ignored.

Include secondary and tertiary text instructions in plain-text maneuver instructions for CarPlay units that lack support for attributed strings.
Downgrade custom text attachment subclasses to NSTextAttachment before adding them to attributed instructions.
@1ec5
Copy link
Contributor Author

1ec5 commented Sep 12, 2018

Confirmed that the shields show up on a CarPlay device, though the generic shields end up illegible in the night style. We’ll figure that out as tail work.

device

@1ec5 1ec5 merged commit a4fb793 into carplay Sep 12, 2018
@1ec5 1ec5 deleted the 1ec5-carplay-shields branch September 12, 2018 21:52
@1ec5 1ec5 added this to the v0.21.0 milestone Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CarPlay Bugs, improvements and feature requests on Apple CarPlay
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants