fix OS X apps code to return failure on failure to launch app

RESOLVED FIXED in mozilla25

Status

()

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jaas, Assigned: jaas)

Tracking

Trunk
mozilla25
All
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

2.76 KB, patch
Felipe
: review+
smichaud
: review+
RyanVM
: checkin+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
Created attachment 745963 [details] [diff] [review]
fix v1.0

I noticed that the success variable for launching an app is being ignored. Upon further inspection it seems like it shouldn't be.

This patch gets rid of the unused variable warning and properly returns success or failure. Updated the JS consumer to handle exceptions properly.
(Assignee)

Comment 1

6 years ago
Comment on attachment 745963 [details] [diff] [review]
fix v1.0

Requesting review from Bill Walker, perhaps Bill can find a proper reviewer for the JS code.
Attachment #745963 - Attachment is patch: true
Attachment #745963 - Attachment mime type: text/x-patch → text/plain
Attachment #745963 - Flags: review?(bwalker)
(Assignee)

Comment 2

6 years ago
Also, this patch switches to using stack-based autorelease pools.
(Assignee)

Updated

6 years ago
Attachment #745963 - Flags: review?(smichaud)
Comment on attachment 745963 [details] [diff] [review]
fix v1.0

This looks fine to me, but have you tested what happens when an app fails to launch?

One test would be to comment out the call to -[NSWorkspace launchAppWithBundleIdentifier:...] and always return NS_ERROR_FAILURE from nsMacWebAppUtils::LaunchAppWithIdentifier().  Another would be to comment out the call to mwaUtils.launchAppWithIdentifier(aData.origin) and fake an exception instead.
Attachment #745963 - Flags: review?(smichaud) → review+
(Assignee)

Updated

6 years ago
Assignee: nobody → joshmoz
(Assignee)

Updated

6 years ago
Duplicate of this bug: 780478
Comment on attachment 745963 [details] [diff] [review]
fix v1.0

Felipe reviewed all the changes to this file that weren't platform-wide refactoring efforts, so he would be a good reviewer for these changes to it.
Attachment #745963 - Flags: review?(bwalker) → review?(felipc)
Attachment #745963 - Flags: review?(felipc) → review+
Comment on attachment 745963 [details] [diff] [review]
fix v1.0

Josh, is this patch ready to land? It was r+'d a month ago. <:)
Attachment #745963 - Flags: checkin?(joshmoz)
Comment on attachment 745963 [details] [diff] [review]
fix v1.0

Just set checkin-needed in the future :)
Attachment #745963 - Flags: checkin?(joshmoz) → checkin+
https://hg.mozilla.org/mozilla-central/rev/8e0fdcc36261
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.