Comment #2 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c2

The following revision refers to this bug:

unread,

Jan 9, 2013, 10:09:44 PM1/9/13

to chromi...@chromium.org

Comment #11 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Pic of Dialog Widget Example from WIP CL:
http://codereview.chromium.org/11818044
This uses the correct (x) close button without a border, and adjusts insets.

Attachments:
idr_a.png 14.2 KB

chro...@googlecode.com

unread,

Jan 11, 2013, 1:09:34 AM1/11/13

to chromi...@chromium.org

Comment #12 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c12

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=176265

------------------------------------------------------------------------
r176265 | m...@chromium.org | 2013-01-11T05:08:56.382750Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/chrome_style.cc?r1=176265&r2=176264&pathrev=176265
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/theme/default_100_percent/web_ui_close.png?r1=176265&r2=176264&pathrev=176265
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/theme/default_200_percent/web_ui_close.png?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/theme/theme_resources.grd?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/web_intent_picker_views.cc?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/window/dialog_frame_view.cc?r1=176265&r2=176264&pathrev=176265
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/theme/default_100_percent/web_ui_close_hover.png?r1=176265&r2=176264&pathrev=176265
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/theme/default_200_percent/web_ui_close_hover.png?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/print_preview/search/destination_search.css?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/print_preview/no_destinations_promo.css?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/chromeos/login/oobe.css?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/hover_close_button.mm?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/shared/css/overlay.css?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/resources/shared/css/dialogs.css?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/confirm_bubble_views.cc?r1=176265&r2=176264&pathrev=176265
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/controls/button/label_button.cc?r1=176265&r2=176264&pathrev=176265
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/theme/default_100_percent/web_ui_close_pressed.png?r1=176265&r2=176264&pathrev=176265
D
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/theme/default_200_percent/web_ui_close_pressed.png?r1=176265&r2=176264&pathrev=176265

Replace IDR_WEB_UI_CLOSE* with IDR_CLOSE_DIALOG*, cleanup, etc.

Replace IDR_WEB_UI_CLOSE* uses with IDR_CLOSE_DIALOG*.
(same images from ui/resources instead of chrome/app/theme)
Remove IDR_WEB_UI_CLOSE* IDRs and web_ui_close*.png images.

Give ConfirmBubbleViews's close button hover/pressed images.
(this better matches the specification for the new dialog style)
Note: To test the only use of ConfirmBubbleViews:
1) Open a webpage textfield context menu (right-click).
2) Select "Spell-checker options" -> "Ask Google for suggestions".
3) Observe the dialog's close button on hover/press.

Replace the LabelButtonBorder in LabelButton::SetNativeTheme.
(instead of casting a potentially non-existant/different border)
Cap the label width to a non-negative size in LabelButton::Layout.
(to prevent crashes when the image size exceeds the available size)

Remove the DialogFrameView close button border, adjust Layout.
(this better matches the specification for the new dialog style)
See the resulting screenshot at: http://crbug.com/166075#c11

BUG=166075
TEST=Spelling bubble has better (x) image; no build breaks.
R=s...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/11818044
------------------------------------------------------------------------

chro...@googlecode.com

unread,

Jan 17, 2013, 5:21:30 AM1/17/13

to chromi...@chromium.org

Comment #13 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Pic of Task Manager on Windows with --enable-new-dialog-style from Patch
Set 1 of WIP CL: https://codereview.chromium.org/11975038/
TODO: Use table_view_views or support transparency with table_view_win.
TODO: Use new button style for "End Process" and "Purge Memory" buttons.

Attachments:
task_manager_style_a.tiff 16.8 KB

chro...@googlecode.com

unread,

Jan 17, 2013, 6:18:49 AM1/17/13

to chromi...@chromium.org

Comment #14 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles

task_manager_style_a.png 17.8 KB

chro...@googlecode.com

unread,

Jan 17, 2013, 6:19:49 AM1/17/13

to chromi...@chromium.org

Comment #15 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Pic of similar with "Chrome Style" "End Process" TextButton from Patch Set
2 of WIP CL: https://codereview.chromium.org/11975038/

chro...@googlecode.com

unread,

Jan 17, 2013, 6:20:49 AM1/17/13

to chromi...@chromium.org

Comment #16 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles

http://code.google.com/p/chromium/issues/detail?id=166075

Pic of similar with "Chrome Style" "End Process" TextButton from Patch Set
2 of WIP CL: https://codereview.chromium.org/11975038/

Attachments:
task_manager_style_b.png 17.9 KB

chro...@googlecode.com

unread,

Jan 18, 2013, 6:43:57 AM1/18/13

to chromi...@chromium.org

Comment #18 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Pics Task Manager and Edit Bookmark with --enable-new-dialog-style from
Patch Set 1 of WIP CL: https://codereview.chromium.org/12026016/

Attachments:
task_manager_style_d.png 0 bytes
edit_bookmark_f.png 49.2 KB

chro...@googlecode.com

unread,

Jan 18, 2013, 6:46:18 AM1/18/13

to chromi...@chromium.org

Comment #19 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Pics of Task Manager and Edit Bookmark with --enable-new-dialog-style from

chro...@googlecode.com

unread,

Jan 18, 2013, 6:52:29 AM1/18/13

to chromi...@chromium.org

Comment #20 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Odd that file won't upload, but looks fine locally. Here's another
task_manager_d.png.

Attachments:
task_manager_d.png 69.3 KB

chro...@googlecode.com

unread,

Jan 18, 2013, 4:46:01 PM1/18/13

to chromi...@chromium.org

Comment #21 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c21

The following revision refers to this bug:

unread,

Feb 1, 2013, 8:36:21 PM2/1/13

to chromi...@chromium.org

Comment #27 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c27

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=180208

------------------------------------------------------------------------
r180208 | s...@chromium.org | 2013-02-02T00:16:29.872277Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_browser_ui.gypi?r1=180208&r2=180207&pathrev=180208
A
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/simple_message_box_win.cc?r1=180208&r2=180207&pathrev=180208

Revert 179608

> views: Remove simple_message_box_win.cc in favor of
> simple_message_box_views.cc

> simple_message_box_win.cc is not be needed nor compatible with the
> desktop-aura
> transition. It also uses the win32 appearance not the new Chrome style
> appearance currently in development. So it's bad twice ;)

> Just kill it and that eases the work that will need to be done later.

> By enabling simple_message_box_views on Windows means that it will be
> used for all
> ShowMessageBox() dialogs instead of current Windows MessageBox.

> BUG=166075,54988
> R=b...@chromium.org

> Review URL: https://codereview.chromium.org/12042083

Reverting as causes crashes on windows. Specifically this code is
not necessarily executed on the UI thread and views is not designed to
be used on anything but the UI thread.

TBR=tfa...@chromium.org
BUG=173751
Review URL: https://codereview.chromium.org/12173002
------------------------------------------------------------------------

chro...@googlecode.com

unread,

Feb 4, 2013, 3:56:57 PM2/4/13

to chromi...@chromium.org

Comment #28 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c28

The following revision refers to this bug:

unread,

Mar 20, 2013, 3:30:50 PM3/20/13

to chromi...@chromium.org

Comment #37 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Pics of WIP CL https://codereview.chromium.org/12391056 Patch Set 1:

Attachments:
dialog_widget_cros_before.png 32.3 KB
dialog_widget_cros_after.png 27.9 KB

chro...@googlecode.com

unread,

Mar 22, 2013, 3:35:35 PM3/22/13

to chromi...@chromium.org

Comment #41 on issue 166075 by stro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Just as a followup, yes it is intentional. Apparently there are weird
padding and spacing issues that appear when you align it with the title.

chro...@googlecode.com

unread,

Mar 22, 2013, 3:54:35 PM3/22/13

to chromi...@chromium.org

Comment #42 on issue 166075 by m...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Really? I'm not aware of any technical limitations to vertically aligning
the title and the (x) button.
I was just following the mocks
https://docs.google.com/a/google.com/folder/d/0B6x6iYCtKinEUTFOWUJFTWJFSTA/edit?docId=0B6x6iYCtKinENmVDdWxJZEh5dDQ

chro...@googlecode.com

unread,

Mar 22, 2013, 4:27:35 PM3/22/13

to chromi...@chromium.org

Comment #43 on issue 166075 by stro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075

Sorry I should have been more clear. From my understanding it was a visual
design choice, not a technical limitation.

chro...@googlecode.com

unread,

Mar 23, 2013, 12:42:23 PM3/23/13

to chromi...@chromium.org

Comment #44 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c44

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=190030

------------------------------------------------------------------------
r190030 | m...@chromium.org | 2013-03-23T16:39:00.268843Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/window/dialog_delegate.cc?r1=190030&r2=190029&pathrev=190030

Remove Aura's extra shadowed border in the new dialog style.

Set new Aura dialogs' corewm::ShadowType to SHADOW_TYPE_NONE.
Removes the extra frame around new style dialogs on CrOS.
( does the same on the experimental Windows ash_shell )
See http://crbug.com/166075#c37 for before/after screenshots.

This doesn't remove the black dialog border on Win.
( non-transparency is required to host native controls )

BUG=166075
TEST=CrOS --enable-new-dialog-style looks nicer.
R=s...@chromium.org,b...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/12391056
------------------------------------------------------------------------

chro...@googlecode.com

unread,

Apr 16, 2013, 12:45:56 AM4/16/13

to chromi...@chromium.org

Comment #49 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c49

------------------------------------------------------------------------
r194292 | m...@chromium.org | 2013-04-16T04:40:54.788117Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/window/dialog_delegate.cc?r1=194292&r2=194291&pathrev=194292
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/controls/textfield/textfield.cc?r1=194292&r2=194291&pathrev=194292

Enable new dialog style transparency with Views Textfields.

This makes new-style Dialog widgets transparent on non-Aura Win.
Use Views Textfields if the new dialog style switch is specified.
( transparency prohibits hosting native Windows textfield controls )
( new-dialog-style + win-textfields is possible but breaks dialog
textfields )
See the border of before/after pics at http://crbug.com/166075#c46

BUG=166075,231012
TEST=Non-Aura Win new-style dialogs don't have any extra black/glass border.
R=s...@chromium.org

Review URL: https://codereview.chromium.org/14049016

chro...@googlecode.com

unread,

Apr 23, 2013, 11:39:17 PM4/23/13

to chromi...@chromium.org

Comment #51 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c51

------------------------------------------------------------------------
r196008 | m...@chromium.org | 2013-04-24T02:38:20.597259Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/controls/button/label_button.cc?r1=196008&r2=196007&pathrev=196008

Restrict the new button style to the new dialog style flag.

BUG=155363,166075
TEST=The new button style is only used with --enable-new-dialog-style.
R=s...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/14118013

chro...@googlecode.com

unread,

May 6, 2013, 4:20:17 PM5/6/13

to chromi...@chromium.org

Comment #54 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c54

------------------------------------------------------------------------
r198506 | to...@chromium.org | 2013-05-06T19:30:14.401603Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/window/dialog_delegate.h?r1=198506&r2=198505&pathrev=198506
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/bubble/bubble_border.cc?r1=198506&r2=198505&pathrev=198506
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/bubble/bubble_border.h?r1=198506&r2=198505&pathrev=198506
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/bubble/bubble_frame_view.cc?r1=198506&r2=198505&pathrev=198506
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/constrained_window_views.cc?r1=198506&r2=198505&pathrev=198506
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/window/dialog_delegate.cc?r1=198506&r2=198505&pathrev=198506

Revert 198491 "Render opaque border with no shadow for web conte..."
Broke the chromiumos bots:
http://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20%28daisy%29/builds/9294/steps/cbuildbot/logs/stdio

> Render opaque border with no shadow for web contents modal dialogs under
> Views/Win32

> This is a temporary kludge to get web contents modal dialog frames
> displaying acceptably under Views/Win32. It's not possible to use
> transparency in the frames on Win7 and earlier OSes unless the web
> contents modal dialogs are top-level windows, which is undesirable.
> Making web contents modal dialogs top-level imposes additional
> complexities and burdens on managing focus, activation, and window
> position relative to the browser window, soley for the View/Win32 case.
> It's best to avoid this since we won't need it once we transition
> to Aura.

> See screenshots at http://crbug.com/231012#c6.

> BUG=231012, 166075

> Review URL: https://chromiumcodereview.appspot.com/14742002

TBR=wit...@chromium.org

Review URL: https://codereview.chromium.org/14765016

chro...@googlecode.com

unread,

May 7, 2013, 12:11:19 AM5/7/13

See before/after pics (on Win) at http://crbug.com/166075#c68
(I can't trigger on CrOS ToT, oshima suspects a regression)
Trigger the dialog on Win by clicking a URI like the "spotify:" one at:
https://www.spotify.com/us/blog/archives/2008/01/14/linking-to-spotify/

BUG=166075
TEST=new-style external procol dialog does not have a black border.
R=s...@chromium.org,osh...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/14987006
------------------------------------------------------------------------

chro...@googlecode.com

unread,

May 14, 2013, 10:53:57 PM5/14/13

to chromi...@chromium.org

Comment #78 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c78

------------------------------------------------------------------------
r200137 | m...@chromium.org | 2013-05-15T01:28:44.189634Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/download/download_in_progress_dialog_view.cc?r1=200137&r2=200136&pathrev=200137
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/download/download_in_progress_dialog_view.h?r1=200137&r2=200136&pathrev=200137

Remove the download in progress dialog black border.

DownloadInProgressDialogView uses Widget::CreateWindowWithParent.

Use DialogDelegate::CreateDialogWidget instead.
Does not change the old-style appearance or behavior.

See before/after pics at http://crbug.com/166075#c70
Trigger the dialog by closing chrome while downloading a file.

BUG=166075
TEST=new-style download in progress dialog does not have a black border.
R=s...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/15060010

chro...@googlecode.com

unread,

May 15, 2013, 5:33:38 AM5/15/13

to chromi...@chromium.org

unread,

May 29, 2013, 3:03:31 PM5/29/13

to chromi...@chromium.org

Comment #109 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog
Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c109

------------------------------------------------------------------------
r202913 | m...@chromium.org | 2013-05-29T18:12:44.330414Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/crypto_module_password_dialog_view.cc?r1=202913&r2=202912&pathrev=202913

Remove the crypto module password dialog extra border.

CryptoModulePasswordDialogView uses Widget::CreateWindow.
Use DialogDelegate::CreateDialogWidget instead.
See before/after pics at http://crbug.com/166075#c107

Trigger with debug code in Patch Set 1.

BUG=166075

TEST=Crtypo module password dialog does not have an extra border.
R=s...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/15673004

chro...@googlecode.com

unread,

May 30, 2013, 2:53:19 AM5/30/13

to chromi...@chromium.org

Comment #111 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog
Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c111

------------------------------------------------------------------------
r203092 | m...@chromium.org | 2013-05-30T06:02:11.605536Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/ash/shell.cc?r1=203092&r2=203091&pathrev=203092

Use CustomFrameViewAsh in ash::Shell::CreateDefaultNonClientFrameView.

Misc Ash windows incorrectly get new style BubbleFrameViews.
Revert new dialog style change to CreateDefaultNonClientFrameView.
This reverts non-dialog Ash windows to the old CustomFrameViewAsh.
See before/after pics at http://crbug.com/241810#c17

Dialogs get frames from DialogDelegate::CreateNonClientFrameView.
( where 'dialogs' are DialogDelegate[View] [subclass] instances )

BUG=241125,241810,166075
TEST=Only dialog windows have the new-style bubble border. No windows have
a double-border issue.
R=de...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/15685017

chro...@googlecode.com

unread,

May 31, 2013, 3:23:23 AM5/31/13

to chromi...@chromium.org

Comment #113 on issue 166075 by bugdro...@chromium.org: Cleanup Dialog
Styles
http://code.google.com/p/chromium/issues/detail?id=166075#c113

------------------------------------------------------------------------
r203354 | m...@chromium.org | 2013-05-31T07:18:28.798659Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/bubble/bubble_frame_view.cc?r1=203354&r2=203353&pathrev=203354
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/bubble/bubble_delegate.cc?r1=203354&r2=203353&pathrev=203354
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/bubble/bubble_frame_view_unittest.cc?r1=203354&r2=203353&pathrev=203354

Restore Views dialog dragging and titlebar system menus.

Allow non-resizable dialog dragging (and system menu).
(revise NonClientHitTest to return HTCAPTION as needed)
This was a regression from my http://crrev.com/202000

Allow dragging dialogs from the frame view side/botttom.
This is new behavior that I think is good :)

Expand BubbleFrameViewTest.NonClientHitTest and support.
Pass through BubbleBorderDelegate's CanResize logic.
(really just needed temporarily for the added unit testing)

BUG=244676,166075
TEST=all non-constrained dialogs can be dragged, and their Windows system
menus are shown on title-bar right click.
R=s...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/16084008

chro...@googlecode.com

unread,

Reply to author

Forward