New issue
Advanced search Search tips

Issue 767743 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug

Blocking:
issue 671916



Sign in to add a comment

MacViewsBrowser: WebModals shouldn't animate their window when switching tabs

Project Member Reported by tapted@chromium.org, Sep 22 2017

Issue description

Chrome Version       : 63.0.3213.3

E.g.
 - Open http://httpbin.org/basic-auth/user/passwd, dialog animates in
 - open a new tab, switch back

dialog should just reappear.

In mac_views_browser it doesn't, it animates again.

It happens for Cocoa dialogs since they are never ordered out - the windows are just made fully transparent. That's hacky, and we probably can't do the same long-term.

https://chromium-review.googlesource.com/c/chromium/src/+/676550 is adding code for views-dialogs-on-cocoa-browser to do something in between. Solution might involve borrowing some code from that.
 
Labels: MacViews-Browser Target-68
Owner: robliao@chromium.org
Status: Assigned (was: Available)
MacViews triage: this repros for me; assigning to robliao@ for M-68.

Comment 2 by gov...@chromium.org, Mar 27 2018

Labels: M-68

Comment 3 by gov...@chromium.org, Mar 29 2018

** Bulk Edit **

FYI: Starting 04/13 M68 will be in canary, M68 Dev promotion will be on 04/26.

Comment 4 by gov...@chromium.org, Apr 25 2018

Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.
Labels: -Target-68 Target-69
Labels: -M-68
Labels: M-68
Labels: Group-Views_Regressions_from_Cocoa
Cc: robliao@chromium.org
Labels: -Pri-3 Pri-1
Owner: tapted@chromium.org
Status: Started (was: Assigned)
https://chromium-review.googlesource.com/1146408
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f4f6cf36f5896076ac15961149fbfd6f06fe078

commit 7f4f6cf36f5896076ac15961149fbfd6f06fe078
Author: Trent Apted <tapted@chromium.org>
Date: Mon Jul 23 23:50:01 2018

MacViews: Don't animate in web-modals after the first Show().

Implement the currently unimplemented (on mac) NativeWidget::
SetVisibilityAnimationTransition() for this. Autofill uses this
but not constrained windows.

Constrained windows on aura platforms have bespoke animation
suppression code, which is currently different to the desired
behavior on Mac (i.e. the very first "show" should animate).

Consolidate some of the animation logic in MacViews Widgets.

Bug:  767743 
Change-Id: If49d139a66873e896e68030f69efdaa81a8dc948
Reviewed-on: https://chromium-review.googlesource.com/1146408
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577343}
[modify] https://crrev.com/7f4f6cf36f5896076ac15961149fbfd6f06fe078/components/constrained_window/native_web_contents_modal_dialog_manager_views.cc
[modify] https://crrev.com/7f4f6cf36f5896076ac15961149fbfd6f06fe078/ui/views/cocoa/bridged_native_widget.h
[modify] https://crrev.com/7f4f6cf36f5896076ac15961149fbfd6f06fe078/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/7f4f6cf36f5896076ac15961149fbfd6f06fe078/ui/views/widget/native_widget_mac.mm
[modify] https://crrev.com/7f4f6cf36f5896076ac15961149fbfd6f06fe078/ui/views/widget/native_widget_mac_unittest.mm

Labels: Merge-Request-69
Requesting merge for r577343 . Tested 70.0.3501.2 using print preview and https://httpbin.org/#/Auth/get_basic_auth__user___passwd_ - fix is working as intended.
Project Member

Comment 12 by sheriffbot@chromium.org, Jul 25

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 26

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/020c1c4d433ca196fed3a15157f42d9bfd906515

commit 020c1c4d433ca196fed3a15157f42d9bfd906515
Author: Trent Apted <tapted@chromium.org>
Date: Thu Jul 26 00:03:56 2018

[merge-m69] MacViews: Don't animate in web-modals after the first Show().

Implement the currently unimplemented (on mac) NativeWidget::
SetVisibilityAnimationTransition() for this. Autofill uses this
but not constrained windows.

Constrained windows on aura platforms have bespoke animation
suppression code, which is currently different to the desired
behavior on Mac (i.e. the very first "show" should animate).

Consolidate some of the animation logic in MacViews Widgets.

TBR=tapted@chromium.org

(cherry picked from commit 7f4f6cf36f5896076ac15961149fbfd6f06fe078)

Bug:  767743 
Change-Id: If49d139a66873e896e68030f69efdaa81a8dc948
Reviewed-on: https://chromium-review.googlesource.com/1146408
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577343}
Reviewed-on: https://chromium-review.googlesource.com/1150016
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#94}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/020c1c4d433ca196fed3a15157f42d9bfd906515/components/constrained_window/native_web_contents_modal_dialog_manager_views.cc
[modify] https://crrev.com/020c1c4d433ca196fed3a15157f42d9bfd906515/ui/views/cocoa/bridged_native_widget.h
[modify] https://crrev.com/020c1c4d433ca196fed3a15157f42d9bfd906515/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/020c1c4d433ca196fed3a15157f42d9bfd906515/ui/views/widget/native_widget_mac.mm
[modify] https://crrev.com/020c1c4d433ca196fed3a15157f42d9bfd906515/ui/views/widget/native_widget_mac_unittest.mm

Status: Fixed (was: Started)

Sign in to add a comment