fix: correctly enable and disable windows on Windows and Linux (backport: 3-0-x) (#15255)

* fix: correctly enable and disable windows

* Move has_child_modal_ to NativeWindowViews

* Track modal children as an int instead of a bool

* Use correct types

* Move Increment/DecrementChildModals to NativeWindowViews

* Use parent() in NativeWindowViews

* Add test for not enabling disabled parent when modal child closes
This commit is contained in:
trop[bot] 2018-10-19 17:30:55 -07:00 committed by Shelley Vohr
parent 1c42715e1d
commit 9349b0a273
3 changed files with 54 additions and 17 deletions

View file

@ -342,7 +342,7 @@ bool NativeWindowViews::IsFocused() {
void NativeWindowViews::Show() {
if (is_modal() && NativeWindow::parent() &&
!widget()->native_widget_private()->IsVisible())
NativeWindow::parent()->SetEnabled(false);
static_cast<NativeWindowViews*>(parent())->IncrementChildModals();
widget()->native_widget_private()->ShowWithWindowState(GetRestoredState());
@ -367,7 +367,7 @@ void NativeWindowViews::ShowInactive() {
void NativeWindowViews::Hide() {
if (is_modal() && NativeWindow::parent())
NativeWindow::parent()->SetEnabled(true);
static_cast<NativeWindowViews*>(parent())->DecrementChildModals();
widget()->Hide();
@ -391,16 +391,34 @@ bool NativeWindowViews::IsEnabled() {
#endif
}
void NativeWindowViews::IncrementChildModals() {
num_modal_children_++;
SetEnabledInternal(ShouldBeEnabled());
}
void NativeWindowViews::DecrementChildModals() {
if (num_modal_children_ > 0) {
num_modal_children_--;
}
SetEnabledInternal(ShouldBeEnabled());
}
void NativeWindowViews::SetEnabled(bool enable) {
// Handle multiple calls of SetEnabled correctly.
if (enable) {
--disable_count_;
if (disable_count_ != 0)
return;
} else {
++disable_count_;
if (disable_count_ != 1)
return;
if (enable != is_enabled_) {
is_enabled_ = enable;
SetEnabledInternal(ShouldBeEnabled());
}
}
bool NativeWindowViews::ShouldBeEnabled() {
return is_enabled_ && (num_modal_children_ == 0);
}
void NativeWindowViews::SetEnabledInternal(bool enable) {
if (enable && IsEnabled()) {
return;
} else if (!enable && !IsEnabled()) {
return;
}
#if defined(OS_WIN)
@ -1155,10 +1173,10 @@ void NativeWindowViews::OnWidgetBoundsChanged(views::Widget* changed_widget,
}
void NativeWindowViews::DeleteDelegate() {
if (is_modal() && NativeWindow::parent()) {
auto* parent = NativeWindow::parent();
if (is_modal() && this->parent()) {
auto* parent = this->parent();
// Enable parent window after current window gets closed.
parent->SetEnabled(true);
static_cast<NativeWindowViews*>(parent)->DecrementChildModals();
// Focus on parent window.
parent->Focus(true);
}

View file

@ -129,6 +129,9 @@ class NativeWindowViews : public NativeWindow,
void UpdateDraggableRegions(std::unique_ptr<SkRegion> region);
void IncrementChildModals();
void DecrementChildModals();
#if defined(OS_WIN)
void SetIcon(HICON small_icon, HICON app_icon);
#elif defined(USE_X11)
@ -185,6 +188,10 @@ class NativeWindowViews : public NativeWindow,
LPARAM l_param);
#endif
// Enable/disable:
bool ShouldBeEnabled();
void SetEnabledInternal(bool enabled);
// NativeWindow:
void HandleKeyboardEvent(
content::WebContents*,
@ -268,8 +275,11 @@ class NativeWindowViews : public NativeWindow,
// has to been explicitly provided.
std::unique_ptr<SkRegion> draggable_region_; // used in custom drag.
// How many times the Disable has been called.
int disable_count_ = 0;
// Whether the window should be enabled based on user calls to SetEnabled()
bool is_enabled_ = true;
// How many modal children this window has;
// used to determine enabled state
unsigned int num_modal_children_ = 0;
bool use_content_size_ = false;
bool movable_ = true;

View file

@ -2747,7 +2747,7 @@ describe('BrowserWindow module', () => {
c.show()
assert.equal(w.isEnabled(), false)
})
it('enables parent window when closed', (done) => {
it('re-enables an enabled parent window when closed', (done) => {
c.once('closed', () => {
assert.equal(w.isEnabled(), true)
done()
@ -2755,6 +2755,15 @@ describe('BrowserWindow module', () => {
c.show()
c.close()
})
it('does not re-enable a disabled parent window when closed', (done) => {
c.once('closed', () => {
assert.strictEqual(w.isEnabled(), false)
done()
})
w.setEnabled(false)
c.show()
c.close()
})
it('disables parent window recursively', () => {
let c2 = new BrowserWindow({show: false, parent: w, modal: true})
c.show()