Broken AppKit behaviour when calling orderOut: on a window with an attachedSheet |
||||
Issue descriptionChrome Version : 56.0.2914.3 OS Version: OS X 10.12.1 Basically, calling orderOut: on a window with an attached sheet causes the sheet to become "detached", and (unlike regular child windows), you can't "reattach" a sheet. We can't even make a sheet a child window because of Issue 605098 . Luckily, we don't tend to call orderOut on browser windows. We do on App Windows, but they are deprecated on Mac. E.g. What steps will reproduce the problem? 1. Install, e.g., the "Caret" Packaged App: https://chrome.google.com/webstore/detail/caret/fljalecfjciodhpcledpamjachpmelml?hl=en 2. Launch Caret, and click "Save" from the File menu (leave the Save dialog open). 3. Right-click Caret's Dock icon and select 'Hide' 4. Click Caret's Dock icon again to show it (Save sheet has disappeared) 5. Try to save again What is the expected result? Save dialog should get shown again What happens instead of that? Nothing happens. Save dialog can't show because it's "already" showing, but AppKit lost it. You can attempt to re-show the sheet. In which case you get something like the attached screenshot -- a sheet that has become detached and is now a draggable window with a titlebar.
,
Nov 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9464d37d42d7b7895421a05bd3f0b5f0a3eb3266 commit 9464d37d42d7b7895421a05bd3f0b5f0a3eb3266 Author: tapted <tapted@chromium.org> Date: Tue Nov 29 04:02:54 2016 MacViews: Protect against orderOut: on an NSWindow with an attached sheet. This fixes and re-enables NativeWidgetMacTest.WindowModalSheet with some additional test expectations. It was reverted in r433028 when it started failing. Fundamentally, the problem is that calling orderOut: on an NSWindow with an attached sheet results in utterly broken behavior in AppKit, documented in http://crbug.com/667602. AppKit's broken behaviour with orderOut and regular child windows is already handled by BridgedNativeWidget by recreating the parent/child relationships. However, it can't do this for sheets. It's not possible to "reattach" a sheet once it becomes detached. And even giving a sheet a parentWindow results in graphical glitches ( http://crbug.com/605098 ). Sheets use -[NSWindow sheetParent], not -[NSWindow parentWindow]. Note that -[NSApp hide:] works. It's just -[NSWindow orderOut:] that's broken. But we need the triggers from -[NSApp hide:] to be consistent with the different things that need to happen for sheets. Add checks to BridgedNativeWidget to detect attempts to orderOut: a window with an attached sheet. Different solutions need to be sought for these, such as what Chrome already does for the certificate viewer sheet when we make it tab-modal. Update DCHECKs in WidgetOwnerNSWindowAdapter to properly support sheets. This allows NativeWidgetMacTest.WindowModalSheet to be re-enabled. Updates BridgedNativeWidget::OnVisibilityChanged() to properly fix http://crbug.com/605098 . This bug still manifests in Chrome, but only after a call to [NSApp hide:]. Re-showing a window improperly attaches the parent as a parentWindow (rather than only sheetParent), and this is the cause of the graphical glitches. BUG= 666503 , 605098 , 667602 Review-Url: https://codereview.chromium.org/2521753003 Cr-Commit-Position: refs/heads/master@{#434882} [modify] https://crrev.com/9464d37d42d7b7895421a05bd3f0b5f0a3eb3266/ui/views/cocoa/bridged_native_widget.mm [modify] https://crrev.com/9464d37d42d7b7895421a05bd3f0b5f0a3eb3266/ui/views/cocoa/widget_owner_nswindow_adapter.mm [modify] https://crrev.com/9464d37d42d7b7895421a05bd3f0b5f0a3eb3266/ui/views/widget/native_widget_mac_unittest.mm
,
Oct 31 2017
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9bac37b48f9edbedef1856c77c0e47296579b5cd commit 9bac37b48f9edbedef1856c77c0e47296579b5cd Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Fri Apr 20 17:36:41 2018 macviews: remove attachedSheet DCHECK This DCHECK is testing for a real problem, but in a way that breaks a lot of test cases. The problem does need to be addressed (see the linked bug) but this DCHECK is not the right approach. Bug: 834926,667602 Change-Id: I3735030562eecca32a9d48135285eb645e49e89b Reviewed-on: https://chromium-review.googlesource.com/1021836 Reviewed-by: Sidney San Martín <sdy@chromium.org> Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#552371} [modify] https://crrev.com/9bac37b48f9edbedef1856c77c0e47296579b5cd/ui/views/cocoa/bridged_native_widget.mm
,
Apr 23 2018
Tried checking the issue on latest chrome version 68.0.3404.0 using Mac 10.13.1 with the exact steps mentioned below. 1. Installed "Caret" from link provided. 2. Launched Caret, and click "Save" from the File menu. 3. Right-clicked on Caret's Dock icon and selected 'Hide' 4. Clicked Caret's Dock icon again to show it. 5. Tried saving it again. We didn't observe save dialogue being prompted. Attaching screen cast of the same for reference. @Elly Fong-Jones: Could you please let us know if we have missed anything in the process. And please let us know if there are any fixes to be landed. Thanks!
,
Nov 26
*** UI Mass Triage *** |
||||
►
Sign in to add a comment |
||||
Comment 1 by ajha@chromium.org
, Nov 22 2016