New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 605008 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Long OOO (go/where-is-mgiuca)
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

BookmarkAppConfirmationView is non-modal (DCHECKs upon opening)

Project Member Reported by mgiuca@chromium.org, Apr 20 2016

Issue description

Version: r387838 (52.0.2711.0)
OS: Linux

What steps will reproduce the problem?
(1) Chrome menu -> More tools -> Add to desktop.
(2) Click / drag the page behind the dialog.

What is the expected output?
Add to Desktop dialog is modal; clicking on page behind has no effect.

(Actually, prior to r387104 it just dismissed the dialog, but I think having it be modal makes more sense.)

What do you see instead?
In a Debug build or Release-with-DCHECKs, opening the window DCHECK-fails:
[11759:11759:0420/164350:FATAL:constrained_window_views.cc(163)] Check failed: ui::MODAL_TYPE_NONE != dialog->GetModalType() (0 vs. 0)
#0 0x7f2b20cda94e base::debug::StackTrace::StackTrace()
#1 0x7f2b20cf9eeb logging::LogMessage::~LogMessage()
#2 0x7f2b22242b0a constrained_window::CreateBrowserModalDialogViews()
#3 0x7f2b221846f7 BookmarkAppConfirmationView::CreateAndShow()
#4 0x7f2b21a8d9c1 extensions::BookmarkAppHelper::OnIconsDownloaded()

In a Release build, the page behind is interactive (the dialog is non-modal).

BookmarkAppConfirmationView needs to override WidgetDelegate::GetModalType to return MODAL_TYPE_WINDOW.

This was broken in r387104 (https://codereview.chromium.org/1878963003) which rewrote the bookmark dialog with BookmarkAppConfirmationView.
 

Comment 1 by mgiuca@chromium.org, Apr 20 2016

It's a quick fix so I just uploaded it myself: https://codereview.chromium.org/1906433002
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 21 2016

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

commit e628940221550b9b7c81440c5fd65f852fbb174b
Author: mgiuca <mgiuca@chromium.org>
Date: Thu Apr 21 00:29:58 2016

Views: Add to desktop dialog is now modal.

Previously, it allowed interaction with the page underneath, and
DCHECK-failed upon opening.

BUG= 605008 

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

Cr-Commit-Position: refs/heads/master@{#388636}

[modify] https://crrev.com/e628940221550b9b7c81440c5fd65f852fbb174b/chrome/browser/ui/views/extensions/bookmark_app_confirmation_view.cc
[modify] https://crrev.com/e628940221550b9b7c81440c5fd65f852fbb174b/chrome/browser/ui/views/extensions/bookmark_app_confirmation_view.h

Comment 3 by mgiuca@chromium.org, Apr 21 2016

Status: Fixed (was: Started)

Sign in to add a comment