Re: Issue 242284 in chromium: ChromeOS: New dialog style problems with chrome::ShowMessageBox

[chro...@googlecode.com]

Comment #5 on issue 242284 by hs...@chromium.org: ChromeOS: New dialog style
problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

Re:#4 I can't seem to find an explicit maximum width defined anywhere. The
chrome::ShowMessageBox() only takes a title and a message. I guess it is up
to the implementation.

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[chro...@googlecode.com]

Comment #6 on issue 242284 by hs...@chromium.org: ChromeOS: New dialog style

problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

msw@ - how far are you from fixing problem #1? If it is not trivial, can we
disable the new style dialog on Chrome OS for now? Thanks

[chro...@googlecode.com]

Comment #8 on issue 242284 by hs...@chromium.org: ChromeOS: New dialog style

problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

We still have a few weeks before the M-29 branch so for now it is not
urgent. But once we branch it will become a fairly visible nuisance.

[chro...@googlecode.com]

Comment #9 on issue 242284 by m...@chromium.org: ChromeOS: New dialog style

problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

I have a CL that fixes the border issue up for review at
https://codereview.chromium.org/15947003/
I'll investigate the title issue next, do you have any suggestions for
triggering a long title (I can just debug too).

[chro...@googlecode.com]

Comment #10 on issue 242284 by hs...@chromium.org: ChromeOS: New dialog

style problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

While you're at it, can you replace the CreateWindowWithParent in
extension_uninstall_dialog_view.cc: ExtensionUninstallDialogViews::Show()
with views::DialogDelegate::CreateDialogWidget() too? (it is shown in the
screenshot #1, somehow it does not go through the ShowMessageBox path).

For the long title, the only one I can find (see screenshot #2) is the
WebRTC screencast confirmation dialog, but it is behind a flag. (You'll
need to enable "enable-usermedia-screen-capture" in about://flags, then
open a WebRTC screencast test page, like
https://www.corp.google.com/~hshi/webrtc/screen.html). It might be easier
if you just subsitute a long title when debugging.

[chro...@googlecode.com]

Comment #11 on issue 242284 by hs...@chromium.org: ChromeOS: New dialog

style problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

Oh sorry never mind, I see you already have a CL under review for #10
(https://codereview.chromium.org/15932002/)

[chro...@googlecode.com]

Comment #12 on issue 242284 by m...@chromium.org: ChromeOS: New dialog style

problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

Yup, but thanks for the tips regardless. AFAIK, the stragglers are
NetworkConfigView and IdleActionWarningDialogView.
Then I'll fix up the title layout/sizing, and evaluate reverting the new
style from ash::Shell::CreateDefaultNonClientFrameView.
Then I'll remove/update about:ipc for Win, support system colors, update
button classes/assets, and just a few hundred more things :)

[chro...@googlecode.com]

Comment #13 on issue 242284 by bugdro...@chromium.org: ChromeOS: New dialog

style problems with chrome::ShowMessageBox

http://code.google.com/p/chromium/issues/detail?id=242284#c13

------------------------------------------------------------------------
r202057 | m...@chromium.org | 2013-05-24T11:26:45.369337Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/simple_message_box_views.cc?r1=202057&r2=202056&pathrev=202057

Remove the message box dialog black border.

SimpleMessageBoxViews uses Widget::CreateWindow.
Use DialogDelegate::CreateDialogWidget instead.
See before/after pics at http://crbug.com/166075#c97
Trigger by one of the following methods (there are many):
1) Open all tabs on a big bookmark folder.
2) encounter extension/profile/print/etc. errors.
See many uses of chrome::ShowMessageBox at:
https://code.google.com/p/chromium/codesearch#search/&sq=package:chromium&q=chrome::ShowMessageBox

BUG=166075,242284
TEST=Message box dialog does not have a black border.
R=s...@chromium.org

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

[chro...@googlecode.com]

Comment #15 on issue 242284 by bugdro...@chromium.org: ChromeOS: New dialog

style problems with chrome::ShowMessageBox

http://code.google.com/p/chromium/issues/detail?id=242284#c15

------------------------------------------------------------------------
r202479 | m...@chromium.org | 2013-05-28T01:56:16.113281Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/bubble/bubble_frame_view.cc?r1=202479&r2=202478&pathrev=202479
M
http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/examples/widget_example.cc?r1=202479&r2=202478&pathrev=202479

Polish new dialog style layout and sizing.

Fix values of top/left dialog title insets, add explicit bottom inset.
Fix dialog inset and preferred size calculations, and layout for
title/extra.
( long titles were not being accounted for, and getting cut off )
Add a title bar extra view to the views examples dialog.
See before/after/spec pics at http://crbug.com/242284#c14

BUG=242284, 240730, 166075
TEST=New dialog style looks good, no titles cut off.
R=s...@chromium.org

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

[chro...@googlecode.com]

Comment #16 on issue 242284 by m...@chromium.org: ChromeOS: New dialog style

problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

AFAIK, the only problem left here is that long JS dialogs are not correctly
centered.
I'll investigate that now, let me know if there is anything else I'm
missing.

[chro...@googlecode.com]

Comment #17 on issue 242284 by m...@chromium.org: ChromeOS: New dialog style

problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

Pics of WIP CL https://codereview.chromium.org/16172006/

Attachments:
long_js_before.png 123 KB
long_js_after.png 92.9 KB

[chro...@googlecode.com]

Updates:
Labels: Cr-Internals-Views

Comment #18 on issue 242284 by tfar...@chromium.org: ChromeOS: New dialog

style problems with chrome::ShowMessageBox
http://code.google.com/p/chromium/issues/detail?id=242284

(No comment was entered for this change.)