Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
On Mon, 6 Jan 2020 21:01:25 GMT, Kevin Rushforth wrote: >> This PR is a fix for >> [JDK-8234474](https://bugs.openjdk.java.net/browse/JDK-8234474), a crash in >> the code that shows a file open or save dialog. >> >> In order to provide additional support for Copy (CMD-C), Cut (CMD-X), and >> Paste (CMD-V), the Glass implementation for displaying a file open or save >> dialog subclasses NSSavePanel or NSOpenPanel to add this support. When the >> application is running in sandboxed mode, the dialogs are shown >> out-of-process by the "powerbox". In this mode, attempting to use our >> subclass results in a security exception. Previously, we added code to >> detect whether we were running in a sandbox as a fix for >> [JDK-8092977](https://bugs.openjdk.java.net/browse/JDK-8092977); we now use >> NSSavePanel or NSOpenPanel directly when in sandboxed mode. >> >> Starting with macOS 10.15 (Catalina) Apple always displays file dialogs >> out-of-process via powerbox, so our use of a subclass is ineffective. >> Further, we have reports of some cases where we crash even though our >> sandbox detection code doesn't indicate that we are running in a sandbox. >> >> Since there is no point in trying to use our subclasses on macOS 10.15 or >> later, I propose to fix this bug by changing the logic so that we use >> NSSavePanel or NSOpenPanel directly in either of the following conditions: >> >> 1) the app is running in sandbox mode >> OR >> 2) The platform is macOS 10.15 or later > > The pull request has been updated with 1 additional commit. - Marked as reviewed by prr (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/70
Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
On Mon, 6 Jan 2020 18:21:17 GMT, Kevin Rushforth wrote: >> Ok, fair enough for 14 ... > > I'll file a follow-up bug for 15 shortly. > > @prrace would you be willing to be the second reviewer on this fix? I filed [JDK-8236685](https://bugs.openjdk.java.net/browse/JDK-8236685) to remove the custom dialogs in JavaFX 15. - PR: https://git.openjdk.java.net/jfx/pull/70
Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
On Mon, 6 Jan 2020 18:19:54 GMT, Phil Race wrote: >> This is targeted for JavaFX 14, which is another reason to go with a more >> targeted (safer) fix, since we are almost at RDP1. For JavaFX 15 it does >> seem best to just remove the functionality entirely. > > Ok, fair enough for 14 ... I'll file a follow-up bug for 15 shortly. @prrace would you be willing to be the second reviewer on this fix? - PR: https://git.openjdk.java.net/jfx/pull/70
Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
On Mon, 6 Jan 2020 18:11:29 GMT, Kevin Rushforth wrote: >> Well then you'll just have to come back to it. If its not critical I'd >> remove it now so that FX 15 (or is this for 14 ?) behaves consistently > > This is targeted for JavaFX 14, which is another reason to go with a more > targeted (safer) fix, since we are almost at RDP1. For JavaFX 15 it does seem > best to just remove the functionality entirely. Ok, fair enough for 14 ... - PR: https://git.openjdk.java.net/jfx/pull/70
Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
On Mon, 6 Jan 2020 17:44:39 GMT, Phil Race wrote: >> This was just added as a convenience, so it isn't critical functionality. >> >> Having said that, while we could just drop the subclasses now, it would >> cause a perceived regression in behavior for users running on macOS 10.14 >> Mojave. So my preference would be to not remove the subclass at this time, >> but to just not use them when running on 10.15 or later. > > Well then you'll just have to come back to it. If its not critical I'd remove > it now so that FX 15 (or is this for 14 ?) behaves consistently This is targeted for JavaFX 14, which is another reason to go with a more targeted (safer) fix, since we are almost at RDP1. For JavaFX 15 it does seem best to just remove the functionality entirely. - PR: https://git.openjdk.java.net/jfx/pull/70
Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
On Mon, 6 Jan 2020 17:35:11 GMT, Kevin Rushforth wrote: >> Based on my reading of https://bugs.openjdk.java.net/browse/JDK-8092977, >> this means the support for edit operations in these dialogs is on its way >> out ... now supportable only in non-sandboxed apps on the older macOS >> releases. Is there a serious practical consequence of this, or was the >> editing just a convenience ? There must have been some resason we went to >> the trouble. However based on this being "on its way out" why try to prolong >> it ? Just drop the subclasses now. > > This was just added as a convenience, so it isn't critical functionality. > > Having said that, while we could just drop the subclasses now, it would cause > a perceived regression in behavior for users running on macOS 10.14 Mojave. > So my preference would be to not remove the subclass at this time, but to > just not use them when running on 10.15 or later. Well then you'll just have to come back to it. If its not critical I'd remove it now so that FX 15 (or is this for 14 ?) behaves consistently - PR: https://git.openjdk.java.net/jfx/pull/70
Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
On Mon, 6 Jan 2020 17:12:45 GMT, Phil Race wrote: >> Looks good to me, Verified that all tests pass on Mojave 10.14.6 > > Based on my reading of https://bugs.openjdk.java.net/browse/JDK-8092977, this > means the support for edit operations in these dialogs is on its way out ... > now supportable only in non-sandboxed apps on the older macOS releases. Is > there a serious practical consequence of this, or was the editing just a > convenience ? There must have been some resason we went to the trouble. > However based on this being "on its way out" why try to prolong it ? Just > drop the subclasses now. This was just added as a convenience, so it isn't critical functionality. Having said that, while we could just drop the subclasses now, it would cause a perceived regression in behavior for users running on macOS 10.14 Mojave. So my preference would be to not remove the subclass at this time, but to just not use them when running on 10.15 or later. - PR: https://git.openjdk.java.net/jfx/pull/70
Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
On Mon, 6 Jan 2020 16:24:33 GMT, Ambarish Rapte wrote: >> The pull request has been updated with 1 additional commit. > > Looks good to me, Verified that all tests pass on Mojave 10.14.6 Based on my reading of https://bugs.openjdk.java.net/browse/JDK-8092977, this means the support for edit operations in these dialogs is on its way out ... now supportable only in non-sandboxed apps on the older macOS releases. Is there a serious practical consequence of this, or was the editing just a convenience ? There must have been some resason we went to the trouble. However based on this being "on its way out" why try to prolong it ? Just drop the subclasses now. - PR: https://git.openjdk.java.net/jfx/pull/70
Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
On Mon, 6 Jan 2020 16:24:43 GMT, Kevin Rushforth wrote: >> This PR is a fix for >> [JDK-8234474](https://bugs.openjdk.java.net/browse/JDK-8234474), a crash in >> the code that shows a file open or save dialog. >> >> In order to provide additional support for Copy (CMD-C), Cut (CMD-X), and >> Paste (CMD-V), the Glass implementation for displaying a file open or save >> dialog subclasses NSSavePanel or NSOpenPanel to add this support. When the >> application is running in sandboxed mode, the dialogs are shown >> out-of-process by the "powerbox". In this mode, attempting to use our >> subclass results in a security exception. Previously, we added code to >> detect whether we were running in a sandbox as a fix for >> [JDK-8092977](https://bugs.openjdk.java.net/browse/JDK-8092977); we now use >> NSSavePanel or NSOpenPanel directly when in sandboxed mode. >> >> Starting with macOS 10.15 (Catalina) Apple always displays file dialogs >> out-of-process via powerbox, so our use of a subclass is ineffective. >> Further, we have reports of some cases where we crash even though our >> sandbox detection code doesn't indicate that we are running in a sandbox. >> >> Since there is no point in trying to use our subclasses on macOS 10.15 or >> later, I propose to fix this bug by changing the logic so that we use >> NSSavePanel or NSOpenPanel directly in either of the following conditions: >> >> 1) the app is running in sandbox mode >> OR >> 2) The platform is macOS 10.15 or later > > The pull request has been updated with 1 additional commit. Looks good to me, Verified that all tests pass on Mojave 10.14.6 - Marked as reviewed by arapte (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/70
Re: [Rev 01] RFR: 8234474: [macos 10.15] Crash in file dialog in sandbox mode
> This PR is a fix for > [JDK-8234474](https://bugs.openjdk.java.net/browse/JDK-8234474), a crash in > the code that shows a file open or save dialog. > > In order to provide additional support for Copy (CMD-C), Cut (CMD-X), and > Paste (CMD-V), the Glass implementation for displaying a file open or save > dialog subclasses NSSavePanel or NSOpenPanel to add this support. When the > application is running in sandboxed mode, the dialogs are shown > out-of-process by the "powerbox". In this mode, attempting to use our > subclass results in a security exception. Previously, we added code to detect > whether we were running in a sandbox as a fix for > [JDK-8092977](https://bugs.openjdk.java.net/browse/JDK-8092977); we now use > NSSavePanel or NSOpenPanel directly when in sandboxed mode. > > Starting with macOS 10.15 (Catalina) Apple always displays file dialogs > out-of-process via powerbox, so our use of a subclass is ineffective. > Further, we have reports of some cases where we crash even though our sandbox > detection code doesn't indicate that we are running in a sandbox. > > Since there is no point in trying to use our subclasses on macOS 10.15 or > later, I propose to fix this bug by changing the logic so that we use > NSSavePanel or NSOpenPanel directly in either of the following conditions: > > 1) the app is running in sandbox mode > OR > 2) The platform is macOS 10.15 or later The pull request has been updated with 1 additional commit. - Added commits: - b13da6f7: Update copyright year to 2020 Changes: - all: https://git.openjdk.java.net/jfx/pull/70/files - new: https://git.openjdk.java.net/jfx/pull/70/files/dca394e6..b13da6f7 Webrevs: - full: https://webrevs.openjdk.java.net/jfx/70/webrev.01 - incr: https://webrevs.openjdk.java.net/jfx/70/webrev.00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/70.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/70/head:pull/70 PR: https://git.openjdk.java.net/jfx/pull/70
