New issue
Advanced search Search tips

Issue 662128 link

Starred by 1 user

Issue metadata

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

Blocked on:
issue 658105
issue 669387
issue 681201

Blocking:
issue 603386


Participants' hotlists:
MacViews-Task-Queue


Sign in to add a comment

MacViews - switch remaining Harmony project dialogs to Views

Project Member Reported by shrike@chromium.org, Nov 3 2016

Issue description

By "Harmony project dialog" I mean all secondary UI dialogs being reworked in the Harmony UI style. That full list lives here:

https://docs.google.com/spreadsheets/d/1xDrxVl--AysmTowWQ7x5XeNVjpomZzQwGX-lhVGdwBA/edit?ts=57c20f87#gid=1131158076&vpid=A1


Right now the #mac-views-webui-dialogs flag switches a subset of the Harmony project dialogs from native Cocoa to Views. There is also the #mac-views-native-dialogs flag which converts other dialogs, including the Task Manager, to Views.

There should be a new flag that, when flipped, converts all Harmony project dialogs from native Cocoa to Views on the Mac. (Note that the Task Manager is not a Harmony dialog.) To get Harmony fully enabled you have to also activate #secondary-ui-md - it would be nice if this new flag enabled this flag also.
 
Labels: Proj-MacViews
Owner: tapted@chromium.org
Status: Assigned (was: Untriaged)
[Chrome Mac Triage] Trent also talked about having a single flag for MacViews at the sync today.

Comment 2 by tapted@chromium.org, Nov 29 2016

Blockedon: 658105
See  Issue 658105  for making everything a single flag (plan: *just* use --secondary-ui-md).

Comment 3 by tapted@chromium.org, Nov 29 2016

Blockedon: 669387

Comment 4 by tapted@chromium.org, Nov 30 2016

There's a WIP change in https://codereview.chromium.org/2539673003/ that converts some remaining dialogs keyed to the `TabDialogs` interface to Harmony. These are
 - Passwords bubble (see  Issue 669387 )
 - Hung renderer dialog (shows up when a render is hung, offers to kill it)
 - Profile chooser view (not sure what this is when invoked from WebContents)
 - Profile signin confirmation (e.g. linking with corp-managed accounts)
 - validation_message_bubble_view.cc (I have no idea what this is :/)

The approach in that WIP CL is a bit too optimistic though - we'll probably need a gradual approach (next step - something more like https://codereview.chromium.org/2534743002/ but I'll probably split that up).

Comment 5 by tapted@chromium.org, Dec 12 2016

Blocking: 603386
Labels: Phase3

Comment 6 by tapted@chromium.org, Dec 19 2016

Blockedon: 675496
Will work on hooking up external protocol request dialog.

Comment 8 by tapted@chromium.org, Jan 13 2017

Blockedon: 681049

Comment 9 by tapted@chromium.org, Jan 13 2017

Blockedon: 681201
Think we should have a field in the spreadsheet to specify whether the dialog is hooked into secondary-ui-md on Mac? Found Some dialogs still needing conversion-
-Confirm Form resubmission.
-Download recovery dialog.

CL for external protocol request dialog - https://codereview.chromium.org/2632653002/
Created this doc - https://docs.google.com/a/google.com/spreadsheets/d/1z-JUX8JXqXIrgXHP5Z4oOSLieL4n0aufE4EB0trKg6E/edit?usp=sharing to track whether we use the Cocoa or the Views version of the dialog with secondary-ui-md.
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 24 2017

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

commit c8473e8aa7a2b5b140bd5e6c77f8abab98adb539
Author: karandeepb <karandeepb@chromium.org>
Date: Tue Jan 24 09:12:25 2017

MacViews: Enable views based External Protocol dialog behind secondary-ui-md flag.

This CL puts the views based External Protocol dialog behind the secondary-ui-md
flag. As a result, the views based version is shown with the flag "secondary-ui-
md" enabled and the Cocoa version is shown without it.

BUG= 662128 ,  652508 

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

[modify] https://crrev.com/c8473e8aa7a2b5b140bd5e6c77f8abab98adb539/chrome/browser/ui/BUILD.gn
[rename] https://crrev.com/c8473e8aa7a2b5b140bd5e6c77f8abab98adb539/chrome/browser/ui/cocoa/external_protocol_dialog_cocoa.mm
[add] https://crrev.com/c8473e8aa7a2b5b140bd5e6c77f8abab98adb539/chrome/browser/ui/cocoa/external_protocol_dialog_views_mac.mm
[modify] https://crrev.com/c8473e8aa7a2b5b140bd5e6c77f8abab98adb539/chrome/browser/ui/views/external_protocol_dialog.cc
[modify] https://crrev.com/c8473e8aa7a2b5b140bd5e6c77f8abab98adb539/chrome/test/BUILD.gn

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 25 2017

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

commit 5fba2166ddb709508371545bdb88a9c87929615d
Author: karandeepb <karandeepb@chromium.org>
Date: Wed Jan 25 11:07:44 2017

MacViews: Enable views based Download Recovery dialog behind secondary-ui-md flag.

This CL puts the views based Download Recovery dialog behind the secondary-ui-md
flag. As a result, the views based version is shown with the flag "secondary-ui-
md" enabled and the Cocoa version is shown without it.

This CL also does some clean-up in download_danger_prompt_browsertest.cc and
parameterizes the test fixture to test it both with and without the secondary-
ui-md flag.

BUG= 662128 , 654142

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

[modify] https://crrev.com/5fba2166ddb709508371545bdb88a9c87929615d/chrome/browser/download/download_danger_prompt.h
[modify] https://crrev.com/5fba2166ddb709508371545bdb88a9c87929615d/chrome/browser/download/download_danger_prompt_browsertest.cc
[modify] https://crrev.com/5fba2166ddb709508371545bdb88a9c87929615d/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/5fba2166ddb709508371545bdb88a9c87929615d/chrome/browser/ui/cocoa/download/download_danger_prompt_impl.cc
[modify] https://crrev.com/5fba2166ddb709508371545bdb88a9c87929615d/chrome/browser/ui/views/download/download_danger_prompt_views.cc

Labels: -Pri-2 MacViews-Dialogs Pri-1
Owner: ellyjo...@chromium.org
I will take on ensuring this gets done.
Draft CL at https://codereview.chromium.org/2834493007/ brings up Zoom bubble on Mac.
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 26 2017

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

commit 948625158c31a467a776ca18e58d470a1c62565d
Author: varkha <varkha@chromium.org>
Date: Wed Apr 26 23:37:10 2017

MacViews: Allows the toolkit-views Zoom Dialog to be used
Updates Mac tests to test both views and Cocoa versions of the Zoom Bubble.

BUG= 662128 ,  177940 
TEST=browser_tests --gtest_filter=*ZoomDecorationTest*
     unit_tests --gtest_filter=*ZoomDecorationTest*

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

[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/browser_dialogs.h
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/cocoa/location_bar/zoom_decoration.h
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/cocoa/location_bar/zoom_decoration.mm
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/cocoa/location_bar/zoom_decoration_browsertest.mm
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/cocoa/location_bar/zoom_decoration_unittest.mm
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/views/browser_dialogs_views_mac.cc
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/views/location_bar/zoom_bubble_view.cc
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/views/location_bar/zoom_bubble_view.h
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/views/location_bar/zoom_bubble_view_browsertest.cc
[modify] https://crrev.com/948625158c31a467a776ca18e58d470a1c62565d/chrome/browser/ui/views/location_bar/zoom_view.cc

Labels: M-64
Blockedon: -675496 -681049
Status: Fixed (was: Assigned)
We're done with this now \o/

Sign in to add a comment