Skip to content

Commit 91f7638

Browse files
fix(docking): guard detached dock area in addDockWidget hot path
- ADS removeDockArea() calls setParent(nullptr) then deleteLater(), leaving a live but orphaned area for one event-loop turn; passing it to addDockWidget() dereferenced a null container and crashed - Introduce DockAreaSanitizer to collapse orphaned areas to nullptr so ADS falls back to the always-valid dock manager as container - Add lastTabClosed signal (type-agnostic, queued) so closing the last browser or mini-app tab respawns "New X" the same way the last editor tab does; wire handleEditorAreaEmptied as single owner - Snapshot initialEditor() before adding any new tab and close it after, preventing mid-flight empty-area respawn in MiniAppManager - Add m_isClosing guard so handleEditorAreaEmptied no-ops during window tear-down - Cover sanitizeDockArea() with a widget-level Qt test that reproduces the exact detach transition without private ADS state
1 parent 857231d commit 91f7638

10 files changed

Lines changed: 417 additions & 46 deletions

src/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ qt_add_executable(NotepadAI
2626
DataPaths.h
2727
DebugManager.h
2828
DefaultDirectoryManager.h
29+
DockAreaSanitizer.h
2930
DockedEditor.h
3031
DockedEditorTitleBar.h
3132

@@ -108,6 +109,7 @@ qt_add_executable(NotepadAI
108109
DataPaths.cpp
109110
DebugManager.cpp
110111
DefaultDirectoryManager.cpp
112+
DockAreaSanitizer.cpp
111113
DockedEditor.cpp
112114

113115
EditorManager.cpp

src/DockAreaSanitizer.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* This file is part of Notepad Next.
3+
* Copyright 2026 NotepadAI contributors
4+
*
5+
* Notepad Next is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* Notepad Next is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with Notepad Next. If not, see <https://www.gnu.org/licenses/>.
17+
*/
18+
19+
#include "DockAreaSanitizer.h"
20+
21+
#include "DockAreaWidget.h"
22+
23+
ads::CDockAreaWidget *sanitizeDockArea(ads::CDockAreaWidget *area)
24+
{
25+
// An area detached from its container (parent already nullptr, deleteLater
26+
// pending) reports dockContainer() == nullptr. Collapse it to nullptr so the
27+
// caller takes ADS's safe add-to-container fallback instead of dereferencing
28+
// a null container. nullptr in -> nullptr out; attached area passes through.
29+
if (area && !area->dockContainer())
30+
return nullptr;
31+
32+
return area;
33+
}

src/DockAreaSanitizer.h

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* This file is part of Notepad Next.
3+
* Copyright 2026 NotepadAI contributors
4+
*
5+
* Notepad Next is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* Notepad Next is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with Notepad Next. If not, see <https://www.gnu.org/licenses/>.
17+
*/
18+
19+
#ifndef DOCKAREASANITIZER_H
20+
#define DOCKAREASANITIZER_H
21+
22+
namespace ads { class CDockAreaWidget; }
23+
24+
// Pure predicate guarding the "feed a dock area to ADS addDockWidget" hot path.
25+
//
26+
// ADS's CDockContainerWidget::removeDockArea() does setParent(nullptr) and THEN
27+
// deleteLater(), so for one event-loop turn a removed dock area is alive (a
28+
// QPointer to it stays non-null) yet orphaned: CDockAreaWidget::dockContainer()
29+
// walks parentWidget() and returns nullptr. Passing such an area as the 3rd arg
30+
// of CDockManager::addDockWidget() makes Container = area->dockContainer() null,
31+
// and the following topLevelDockArea() dereferences that null container — the
32+
// crash recorded in crash_report.txt (Preview reopened while its area was being
33+
// torn down).
34+
//
35+
// sanitizeDockArea() collapses a detached area to nullptr so callers route into
36+
// ADS's add-to-container fallback (Container = the always-valid dock manager),
37+
// which creates a fresh area for the tab. A null input stays null; a still-
38+
// attached area passes through unchanged. Pure and side-effect-free so it is
39+
// unit-testable in isolation (see tests/test_docked_editor_current_area.cpp),
40+
// mirroring the TerminalCwdResolver split.
41+
ads::CDockAreaWidget *sanitizeDockArea(ads::CDockAreaWidget *area);
42+
43+
#endif // DOCKAREASANITIZER_H

src/DockedEditor.cpp

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919

2020
#include "DockedEditor.h"
21+
#include "DockAreaSanitizer.h"
2122
#include "DockAreaTabBar.h"
2223
#include "DockAreaWidget.h"
2324
#include "DockWidgetTab.h"
@@ -92,6 +93,17 @@ DockedEditor::DockedEditor(QWidget *parent) : QObject(parent)
9293
emit editorActivated(editor);
9394
});
9495

96+
// Fire lastTabClosed() once the area is fully empty. dockWidgetRemoved is
97+
// emitted by CDockManager ONLY from CDockWidget::deleteDockWidget() (the
98+
// delete-on-close path) — never during drag/split/float — and AFTER the
99+
// widget and any now-empty dock area are removed, so totalTabCount() reads
100+
// the settled state here. Both tab kinds use DockWidgetDeleteOnClose, so
101+
// neither lingers as a hidden phantom in the layout count.
102+
connect(dockManager, &ads::CDockManager::dockWidgetRemoved, this, [this](ads::CDockWidget *) {
103+
if (totalTabCount() == 0)
104+
emit lastTabClosed();
105+
});
106+
95107
connect(dockManager, &ads::CDockManager::dockAreaCreated, this, [=](ads::CDockAreaWidget* DockArea) {
96108
DockedEditorTitleBar *titleBar = qobject_cast<DockedEditorTitleBar *>(DockArea->titleBar());
97109
connect(titleBar, &DockedEditorTitleBar::doubleClicked, this, &DockedEditor::titleBarDoubleClicked);
@@ -133,6 +145,60 @@ int DockedEditor::count() const
133145
return total;
134146
}
135147

148+
int DockedEditor::totalTabCount() const
149+
{
150+
// Counts every tab of every kind — editors AND nn_previewTab tabs (preview,
151+
// browser, mini-apps, future kinds) — so it answers "is the editor area
152+
// empty?" with no per-type branching: a new tab kind is counted for free.
153+
//
154+
// We iterate dockContainers() (the main dock manager container PLUS every
155+
// floating container), NOT just the main container's dockAreaCount(). Today
156+
// all tab types clear DockWidgetFloatable so none can be torn out into a
157+
// floating window — but counting only the main container would silently
158+
// undercount the instant any future tab type is made floatable, reopening
159+
// the spurious-"New 1" bug in a way that's painful to trace. Iterating all
160+
// containers makes the count correct regardless of the floatable flag.
161+
//
162+
// dockWidgetsCount() (the raw layout count) is correct here rather than
163+
// openDockWidgetsCount() (which filters !isClosed()): both our tab kinds set
164+
// DockWidgetDeleteOnClose, so a closed tab is REMOVED from the layout, never
165+
// left hidden-but-present. There is therefore no closed-but-undeleted
166+
// phantom to overcount, and this avoids a per-widget isClosed() scan. Each
167+
// dockWidgetsCount() is O(1) (a layout count), so this stays a cheap walk.
168+
int total = 0;
169+
170+
for (const ads::CDockContainerWidget *container : dockManager->dockContainers()) {
171+
for (int i = 0; i < container->dockAreaCount(); ++i)
172+
total += container->dockArea(i)->dockWidgetsCount();
173+
}
174+
175+
return total;
176+
}
177+
178+
ScintillaNext *DockedEditor::initialEditor() const
179+
{
180+
// Only a reusable scratch tab if it is the sole editor. count() is
181+
// editors-only (skips nn_previewTab), matching the historic semantics:
182+
// a lone "New X" alongside e.g. a browser tab is still replaceable.
183+
if (count() != 1)
184+
return nullptr;
185+
186+
ScintillaNext *editor = getCurrentEditor();
187+
188+
// getCurrentEditor() can be null mid-close: the cached pointer is auto-
189+
// nulled by QPointer when its editor is destroyed. Treat as "none".
190+
if (editor == nullptr)
191+
return nullptr;
192+
193+
// Reject anything the user might care about: a temporary buffer, a real or
194+
// missing file, or a buffer with undo/redo history (i.e. it was edited).
195+
// Only a truly pristine "New X" survives and may be transparently closed.
196+
if (editor->isTemporary() || editor->isFile() || editor->canUndo() || editor->canRedo())
197+
return nullptr;
198+
199+
return editor;
200+
}
201+
136202
QVector<ScintillaNext *> DockedEditor::editors() const
137203
{
138204
QVector<ScintillaNext *> editors;
@@ -172,7 +238,13 @@ void DockedEditor::dockWidgetCloseRequested()
172238

173239
ads::CDockAreaWidget *DockedEditor::currentDockArea() const
174240
{
175-
return dockManager->focusedDockWidget() ? dockManager->focusedDockWidget()->dockAreaWidget() : latestDockArea.data();
241+
ads::CDockWidget *focused = dockManager->focusedDockWidget();
242+
ads::CDockAreaWidget *area = focused ? focused->dockAreaWidget() : latestDockArea.data();
243+
244+
// Collapse a detached-but-not-yet-destroyed area to nullptr (see
245+
// sanitizeDockArea / DockAreaSanitizer.h for why this prevents the
246+
// null-container crash in CDockManager::addDockWidget).
247+
return sanitizeDockArea(area);
176248
}
177249

178250
void DockedEditor::addEditor(ScintillaNext *editor)

src/DockedEditor.h

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,34 @@ class DockedEditor : public QObject
5252

5353
void switchToEditor(const ScintillaNext *editor);
5454

55+
// Number of real editors only (excludes preview/browser/mini-app tabs).
5556
int count() const;
5657

58+
// Number of tabs of EVERY kind currently in the center dock area —
59+
// editors plus all `nn_previewTab` tabs (preview, browser, mini-apps, and
60+
// any future tab type routed through the same dock manager). Use this, not
61+
// count(), to decide whether the editor area is truly empty (e.g. whether
62+
// to spawn a fresh "New X" buffer). New tab kinds are counted automatically
63+
// as long as they are added through addEditor()/addPreviewTab().
64+
int totalTabCount() const;
65+
66+
// The single reusable "initial" editor — an unedited, pristine "New X"
67+
// scratch buffer that is currently the sole editor — or nullptr if there
68+
// isn't one. "Pristine" means: a New-type buffer (not a file, not missing,
69+
// not temporary) with nothing to undo/redo, i.e. the user never touched it.
70+
//
71+
// This is THE shared definition of "is there a throwaway scratch tab I can
72+
// transparently replace?". Every surface that opens a new piece of content
73+
// (newFile, file open, file preview, web/browser tabs, future tab kinds)
74+
// must snapshot this BEFORE adding its own tab, then close the result AFTER,
75+
// so the scratch "New X" is swapped out without ever emptying the area mid-
76+
// flight (which would otherwise fire lastTabClosed and respawn). Centralised
77+
// here — on DockedEditor, which every tab-spawning subsystem already holds —
78+
// so MainWindow-less callers (e.g. MiniAppManager's web tabs) share it
79+
// instead of reinventing the pristine test. MainWindow::getInitialEditor()
80+
// delegates here.
81+
ScintillaNext *initialEditor() const;
82+
5783
ScintillaNext *previewEditor() const;
5884
void pinPreviewEditor();
5985

@@ -87,6 +113,12 @@ private slots:
87113
void previewTabActivated(QWidget *widget);
88114
void previewEditorSet();
89115

116+
// Emitted after the LAST tab of ANY kind (editor or nn_previewTab) is
117+
// removed from the dock manager, i.e. totalTabCount() just hit 0. The one
118+
// type-agnostic signal callers use to react to "the editor area is now
119+
// empty" — a new tab kind needs no new signal.
120+
void lastTabClosed();
121+
90122
void contextMenuRequestedForEditor(ScintillaNext *editor);
91123
void titleBarDoubleClicked();
92124
};

src/MiniAppManager.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,20 @@ void MiniAppManager::launchApp(const MiniAppDefinition &def)
173173
instance->dockWidget()->setWindowTitle(title);
174174
});
175175

176+
// Snapshot the reusable pristine "New X" scratch tab BEFORE adding the web
177+
// tab; close it AFTER so the area never empties mid-flight (which would fire
178+
// lastTabClosed and respawn). Same swap-out behaviour as opening a file.
179+
ScintillaNext *initialEditor = m_dockedEditor->initialEditor();
180+
176181
// Create tab immediately (shows "Starting..." title)
177182
QWidget *placeholder = new QWidget();
178183
ads::CDockWidget *dw = m_dockedEditor->addPreviewTab(
179184
placeholder, def.name + QStringLiteral(" \xe2\x80\x94 Starting..."), tintedGlobeIcon());
180185
instance->setDockWidget(dw);
181186

187+
if (initialEditor)
188+
initialEditor->close();
189+
182190
// Wire tab close → destroy instance
183191
connect(dw, &ads::CDockWidget::closed, this, [this, instance]() {
184192
instance->destroy();
@@ -395,9 +403,16 @@ void MiniAppManager::launchQuickBrowser(const QUrl &url, bool enableCdp,
395403
if (!webView)
396404
return;
397405

406+
// Snapshot the reusable pristine "New X" scratch tab BEFORE adding the web
407+
// tab; close it AFTER so the area never empties mid-flight. See launchApp().
408+
ScintillaNext *initialEditor = m_dockedEditor->initialEditor();
409+
398410
ads::CDockWidget *dw = m_dockedEditor->addPreviewTab(
399411
webView, url.host().isEmpty() ? url.toString() : url.host(), tintedGlobeIcon());
400412

413+
if (initialEditor)
414+
initialEditor->close();
415+
401416
QuickBrowserTab tab;
402417
tab.webView = webView;
403418
tab.dockWidget = dw;

0 commit comments

Comments
 (0)