+some mac folks, and avi who has done a lot with modality. 

Adding UX folks.

Here's my understanding of the issue and current thoughts after meeting with jlebel@ today. Please correct any misunderstanding and share questions and concerns. 

[0] Summary

If window modal implementation on OSX is reasonably feasible, go for it. If not, this issue might wait for Harmony dialog framework (timing TBD). 

Dialogs in question:
- Dialog A. sign-in dialog
- Dialog B. sync confirmation dialog
- Dialog C. sign-in error dialog

Preventing unexpected dismissal of B and C is desired. This can be done by using a more restricted modality scope than tab modal.

With today's OSX dialog framework, the desired implementation might be tricky and it seems worth assessing the effort and the impact of making dialogs in Sheets. e.g. if it's a many month of work, we might want to wait and use (yet-to-be-implemented) Harmony dialog framework to avoid churns.

If the OSX issue can wait until Harmony dialog comes (timing TBD), in Harmony world, the recommended modality for B and C is *app modal*, and for A, it's *tab modal*. B and C being app modal addresses the same need of window modal i.e. to prevent page navigation and dismissal of the dialogs. For A being tab modal, see [3] below.


Here, a lengthy note for later reference. 

============

[1] The issue:

For these dialogs,

Dialog B. sync confirmation dialog
Dialog C. sign-in error dialog

, there're cases when the dialogs are dismissed since the pages of their owning tabs are navigated to other pages. 

The issue is that the choices on the dialogs should be made by the user via the dialogs, otherwise, the user will be signed out unexpectedly. Using more restricted modality than tab modal can prevent this unexpected page navigation and dialog dismissal.

[2] Current fix:

To address the issue, on Windows/Linux/CrOS, the dialogs have been changed from Tab Modal to Window Modal (often referred as, browser modal) in  crbug.com/663751 .

[3] Note on Sign-in dialog modality:
One thing to note is that, 1st dialog, 

Dialog A. sign-in dialog

, has been turned to Window modal on Windows/Linux/CrOS as part of the same bug.

This might need to be revisited at another time. The user is not signed in yet on Dialog A and there's no need to force a choice at that state. Less blocking modal scope, i.e. tab modal, might be a better fit. (Note: please share if there're reasonings or use cases for this dialog to be a more blocking modal such as window modal or app modal).

[4] This bug: 
This bug is seeking a way to make Dialog B, C, and probably A to window modal on OSX.  

[5] OSX implementation challenges (with current framework without Harmony):
- On OSX, window modal is used for dialogs implemented in Sheets (example: edit bookmark) mostly ( exception: view certificate, which is in tab modal using Sheets). (From a pure guess) it might be tricky to embed webUI-based dialogs, like dialogs A, B, and C, inside Sheets (proper Eng assessment needed). 

- OSX app modal is a floating dialog (not attached to any Chrome's UIs), which is not ideal for these sign-in cases. Also, (from a pure guess again), implementing webUI-based dialog content within the native app modal might be also tricky.

[6] Harmony dialogs/modality:
Harmony is attempting to have two modality scopes (tab, and app) instead of three (tab, window, and app),

- default modal: tab modal  (it's called page modal in Harmony proposal for now, but the modality scope is almost same, i.e. essentially, modal to the web content)
- app modal

Reasoning to eliminate window modal is that the intent for window modal and app modal is likely the same, i.e. to force choice and/or to prevent further navigation, and there's no specific benefit to use Window modal (allow other windows navigable) over App modal (block all navigations) in Chrome context. 

And in Harmony, app modal is intended to be attached to a Chrome's UI and to support dialog contents like Dialog A, B, and C have. 

With that in mind, Harmony's current intent is to move window modal dialogs, e.g. Dialogs B and C, to app modal once Harmony dialog frame work is implemented. Some of window modal dialogs might need to move to tab modal (default) if blocking navigation is unnecessary, e.g. Dialog A. 

Concerning the question [3]: Why did we change the Sign-in dialog (dialog A) to be window modal on Windows?

1. Dialog A is presented in the same flow as dialog B and C, so we thought they need to use the same presentation to ensure they use the same look-and-feel when they are presented. As on windows/linux the presentation of a webcontents dialog and a window modal dialog are the same, this may not have been needed.
2. Dialog A is always presented when a user action. So we assumed it is fine to have it be window modal (similar to other dialogs like "Save page as" or "Add to desktop" dialogs).
3. Ease of code - all these 

Eli: We could revisit this and present Dialog A (the sign-in dialog) as a tab modal if we wanted to.

I've created a patch to fix it: https://codereview.chromium.org/2617583006

This is without my patch:
https://drive.google.com/open?id=0ByXziH_JVCGJdEM5cmptOUFBUzQ

This is with my pathc:
https://drive.google.com/open?id=0ByXziH_JVCGJSkxWUDJzTndvcFk

I didn't show in my video with my patch, but the dialog is window modal. So I can't close without answering the question, I can't close the window, I can't do anything with the window until I answer the dialog (by clicking on Cancel, OK or the X). My patch also modify the first Sign in dialog (where I type my account and my password).

Thanks for the detailed notes Hwi, and thanks for the quick fix Jerome. Given that Jerome already has a CL up, it seems worth the effort to use the Sheets implementation now to make at least dialogues B and C window-modal on Mac.

Regarding whether to make dialogue A window-modal as well, here are the pros/cons I see.

Pros:
- Consistency across the different dialogues during the sign-in flow. This makes implementation easier, and also makes UI/presentation consistent on Mac, where the animation for window-modal Sheets looks different than the animation for tab-modal dialogues.
- Conceptually, sign in to Chrome shouldn't be tied to a given tab, since it's an action that affects your entire profile and isn't spawned from the web contents of any tab.
Cons:
- Window-modal may not be necessary, since the user had to click a button to get the sign-in dialogue to appear, and there's no need to force a choice.

Hwi - would you be okay with us making dialogue A window-modal for now, for ease of implementation and consistency in terms of UX/presentation? Once Harmony rolls out, and there is a more consistent UI for these types of dialogues, we can revisit the decision about dialogue A.

c9: SGTM. I'll make a note on the Harmony dialog list. Thanks!

Actually, one more con to making dialogue A window-modal:
- Sheet is rendered just underneath the bookmarks bar, instead of overlapping it, on Mac.

For that reason alone, we may want to stick with a tab-modal dialogue for A (where the user has to enter their credentials).

I will send a quick summary email to UI review, proposing that we keep dialogue A as tab-modal for the reasons listed above. Jerome, Mihai - let's assume for now that's the direction we'll go with.

It should not be too hard to keep dialogue A as it use to be, and only change dialogue C. I will update the my patch.

Just to confirm: we should update both dialogues B and C (sync confirmation + sign-in error), not just C.

 Issue 679698  has been merged into this issue.

Screenshot for the sync confirmation dialog after https://codereview.chromium.org/2625813003 (window-modal toolkit-views / Harmony dialog (sheet) on Mac).

Command:

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive --dialog=ProfileSigninConfirmationDialogTest.InvokeDialog_default --secondary-ui-md


Need to follow up on a couple of things in that CL though before putting out to review.

Deleted: Screen Shot 2017-01-10 at 20.46.25.png 208 KB

	Screen Shot 2017-01-10 at 20.46.25.png 208 KB View Download

Note that is different than the sync confirmation dialogue (which is attached). The screenshot you've posted is the managed account confirmation dialogue.

The sync confirmation dialogue is implemented in web UI, while the managed account confirmation dialogue is implemented natively.

Deleted: Desktop_Confirmation_Dialogue.png 164 KB

	Desktop_Confirmation_Dialogue.png 164 KB View Download

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

commit c713c645abfea6684e7ed83d48ee7c334b3979dd
Author: msarda <msarda@chromium.org>
Date: Wed Jan 11 12:01:08 2017

Change the views sign-in dialog to be tab modal.

This CL changes the views sign-in dialod to be tab modal and keeps the
views sync confirmation dialog and the sign-in erorr dialogs as window modal.

BUG= 677012 

Review-Url: https://codereview.chromium.org/2619963003
Cr-Commit-Position: refs/heads/master@{#442875}

[modify] https://crrev.com/c713c645abfea6684e7ed83d48ee7c334b3979dd/chrome/browser/ui/signin_view_controller_delegate.h
[modify] https://crrev.com/c713c645abfea6684e7ed83d48ee7c334b3979dd/chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc
[modify] https://crrev.com/c713c645abfea6684e7ed83d48ee7c334b3979dd/chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.h

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

commit 3888fbf6c8994f26f65ba506341a475b0c2f4e1f
Author: tapted <tapted@chromium.org>
Date: Thu Jan 12 02:55:21 2017

MacViews: Harmony for TabDialogs' dialogs (starting with Collected Cookies)

Collected cookies when invoked from the site info bubble (padlock ->
Cookies -> click the "X in use" link) is already hooked up to
--secondary-ui-md. However, it can also be shown via the content
settings bubble when a cookie is blocked.

We also need TabDialogs' Signin confirmation dialog switched to Harmony,
so set up a way to show Harmony dialogs from the TabDialogs interface.

Opt collected cookies into DialogBrowserTest to make it easy to invoke
for testing.

BUG= 654151 ,  677012 , 610428

Review-Url: https://codereview.chromium.org/2534743002
Cr-Commit-Position: refs/heads/master@{#443130}

[delete] https://crrev.com/590f8e6768702a8172c67b3cda7745107b6b1e0d/chrome/browser/collected_cookies_browsertest.cc
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_cocoa.h
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_cocoa.mm
[add] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_views_mac.h
[add] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_views_mac.mm
[add] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/collected_cookies_browsertest.cc
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/test/BUILD.gn

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

commit 972a4d1ae26576f2a0cee1066a6ded6616538641
Author: jlebel <jlebel@chromium.org>
Date: Thu Jan 12 15:51:38 2017

Using native sheet to display modal dialogs for sign in

BUG= 677012 

Review-Url: https://codereview.chromium.org/2617583006
Cr-Commit-Position: refs/heads/master@{#443241}

[modify] https://crrev.com/972a4d1ae26576f2a0cee1066a6ded6616538641/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h
[modify] https://crrev.com/972a4d1ae26576f2a0cee1066a6ded6616538641/chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.mm
[modify] https://crrev.com/972a4d1ae26576f2a0cee1066a6ded6616538641/chrome/browser/ui/signin_view_controller_delegate.h
[modify] https://crrev.com/972a4d1ae26576f2a0cee1066a6ded6616538641/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
[modify] https://crrev.com/972a4d1ae26576f2a0cee1066a6ded6616538641/chrome/browser/ui/sync/one_click_signin_sync_starter.h

side-by-side for the CL in https://codereview.chromium.org/2625813003/ (Cocoa on the right Views/Harmony on the left)

I'm calling it the "Enterprise Signin Confirmation Dialog" (it's profile_signin_confirmation_dialog_* in code).

Deleted: Screen Shot 2017-01-12 at 12.05.44.png 345 KB

	Screen Shot 2017-01-12 at 12.05.44.png 345 KB View Download

Jerome's CL at comment #19 is the final fix for this issue. Just to summarize:
* Dialog A (sing-in dialog) is now tab modal.
* Dialog B (sync confirmation dialog) and C (sign-in error dialog) are now window modal.

I've made  Issue 681049  for converting the native Enterprise Signin Confirmation Dialog to Harmony, and for running it window-modal on Mac.

Verified on Mac OS 10.12.2 using chrome dev M57 #57.0.2984.0 and the fix is working as expected.

Attaching screen-cast for reference.

Adding TE-Verified Labels.

Thanks!

Deleted: 677012.mp4 1.1 MB

	677012.mp4 1.1 MB View Download