Merge pull request #3334 from enquora/integration-test-document-based-app

Gaps exposed by the TextEditor test application and fixed in AppKit, Foundation:

* Foundation: CPUndoManager no longer raises when undo runs in the same run loop cycle as the change it reverts.

* AppKit: The Show Toolbar menu item now reflects the toolbar's visibility.

* AppKit: The Window menu now lists the application's windows, marks the main window and supports Bring All to Front.

AppKit/CPApplication.j

@@ -123,6 +123,9 @@ var CPApplicationDelegate_applicationShouldTerminate_           = 1 << 0,
 
     CPPanel                     _aboutPanel;
 
+    CPMenu                      _windowsMenu;
+    CPMenuItem                  _windowsMenuSeparator;
+
     CPThemeBlend                _themeBlend @accessors(property=themeBlend);
 }
 
@@ -771,11 +774,226 @@ var CPApplicationDelegate_applicationShouldTerminate_           = 1 << 0,
 
         if ([CPPlatform supportsNativeMainMenu])
             window.cpSetMainMenu([self menu]);
+
+        // The new main menu may contain a different Windows menu, so find it again and list
+        // the windows that were ordered in before the main menu was set.
+        _windowsMenu = nil;
+        _windowsMenuSeparator = nil;
+
+        for (var i = 0, count = [_windows count]; i < count; i++)
+        {
+            var aWindow = _windows[i];
+
+            // _windows is indexed by window number and holds a placeholder at index 0.
+            if ([aWindow isKindOfClass:[CPWindow class]])
+                [aWindow _updateWindowsMenuItem];
+        }
     }
     else
         [aMenu _setMenuName:@"CPMainMenu"];
 }
 
+// MARK: Managing the Windows Menu
+
+/*!
+    Returns the application's Windows menu, which lists the application's windows.
+
+    Unless a menu has been set with \c -setWindowsMenu:, this is the submenu of the main menu
+    named \c _CPWindowsMenu. nib2cib gives that name to the menu marked as the Windows menu in
+    Interface Builder.
+    @return the Windows menu, or \c nil if the application has none
+*/
+- (CPMenu)windowsMenu
+{
+    if (!_windowsMenu)
+        _windowsMenu = [[self mainMenu] _menuWithName:@"_CPWindowsMenu"];
+
+    return _windowsMenu;
+}
+
+/*!
+    Makes \c aMenu the application's Windows menu, and moves the items for the listed windows
+    from the previous Windows menu to it.
+    @param aMenu the new Windows menu
+*/
+- (void)setWindowsMenu:(CPMenu)aMenu
+{
+    var previousMenu = [self windowsMenu];
+
+    if (previousMenu === aMenu)
+        return;
+
+    var listedWindows = [self _windowsInWindowsMenu];
+
+    [listedWindows makeObjectsPerformSelector:@selector(_removeFromWindowsMenu)];
+
+    _windowsMenu = aMenu;
+    _windowsMenuSeparator = nil;
+
+    [listedWindows makeObjectsPerformSelector:@selector(_updateWindowsMenuItem)];
+}
+
+/*!
+    Adds an item for \c aWindow to the Windows menu. The item makes the window key and orders it
+    front when chosen. Does nothing if the menu already has an item for \c aWindow.
+    @param aWindow the window to list
+    @param aString the item's title
+    @param isFilename if \c YES, \c aString is a file path, shown as the file name followed by its
+    directory
+*/
+- (void)addWindowsItem:(CPWindow)aWindow title:(CPString)aString filename:(BOOL)isFilename
+{
+    var menu = [self windowsMenu];
+
+    if (!menu || [self _windowsMenuItemForWindow:aWindow])
+        return;
+
+    // Separate the window list from the menu's own items.
+    if (!_windowsMenuSeparator && [menu numberOfItems] > 0)
+    {
+        _windowsMenuSeparator = [CPMenuItem separatorItem];
+        [menu addItem:_windowsMenuSeparator];
+    }
+
+    var item = [[CPMenuItem alloc] initWithTitle:[self _windowsMenuTitle:aString filename:isFilename]
+                                          action:@selector(makeKeyAndOrderFront:)
+                                   keyEquivalent:@""];
+
+    [item setTarget:aWindow];
+    [menu addItem:item];
+
+    [self updateWindowsItem:aWindow];
+}
+
+/*!
+    Changes the title of the Windows menu item for \c aWindow, adding the item if the menu has
+    none for it.
+    @param aWindow the listed window
+    @param aString the item's new title
+    @param isFilename if \c YES, \c aString is a file path, shown as the file name followed by its
+    directory
+*/
+- (void)changeWindowsItem:(CPWindow)aWindow title:(CPString)aString filename:(BOOL)isFilename
+{
+    var item = [self _windowsMenuItemForWindow:aWindow];
+
+    if (item)
+        [item setTitle:[self _windowsMenuTitle:aString filename:isFilename]];
+    else
+        [self addWindowsItem:aWindow title:aString filename:isFilename];
+}
+
+/*!
+    Removes the Windows menu item for \c aWindow, and removes the separator above the window
+    list when the last window item is removed.
+    @param aWindow the window to remove from the list
+*/
+- (void)removeWindowsItem:(CPWindow)aWindow
+{
+    var item = [self _windowsMenuItemForWindow:aWindow];
+
+    if (!item)
+        return;
+
+    var menu = [self windowsMenu];
+
+    [menu removeItem:item];
+
+    if (_windowsMenuSeparator && ![[self _windowsInWindowsMenu] count])
+    {
+        [menu removeItem:_windowsMenuSeparator];
+        _windowsMenuSeparator = nil;
+    }
+}
+
+/*!
+    Updates the state of the Windows menu item for \c aWindow: a check mark for the main window,
+    the mixed state for a miniaturized window, and no mark otherwise.
+    @param aWindow the listed window
+*/
+- (void)updateWindowsItem:(CPWindow)aWindow
+{
+    var item = [self _windowsMenuItemForWindow:aWindow];
+
+    if (!item)
+        return;
+
+    if ([aWindow isMiniaturized])
+        [item setState:CPMixedState];
+    else
+        [item setState:aWindow === [self mainWindow] ? CPOnState : CPOffState];
+}
+
+/*!
+    Orders front every window listed in the Windows menu that is not miniaturized, keeping their
+    order relative to each other.
+    @param aSender the object requesting this
+*/
+- (void)arrangeInFront:(id)aSender
+{
+    var listedWindows = [self _windowsInWindowsMenu],
+        orderedWindows = [self orderedWindows],
+        index = [orderedWindows count];
+
+    // orderedWindows lists front to back, so order front from the back.
+    while (index--)
+    {
+        var aWindow = orderedWindows[index];
+
+        if ([listedWindows containsObjectIdenticalTo:aWindow] && ![aWindow isMiniaturized])
+            [aWindow orderFront:aSender];
+    }
+}
+
+/* @ignore */
+- (CPMenuItem)_windowsMenuItemForWindow:(CPWindow)aWindow
+{
+    var items = [[self windowsMenu] itemArray],
+        count = [items count];
+
+    for (var i = 0; i < count; i++)
+    {
+        var item = items[i];
+
+        if ([item target] === aWindow && [item action] === @selector(makeKeyAndOrderFront:))
+            return item;
+    }
+
+    return nil;
+}
+
+/* @ignore */
+- (CPArray)_windowsInWindowsMenu
+{
+    var items = [[self windowsMenu] itemArray],
+        count = [items count],
+        listedWindows = [];
+
+    for (var i = 0; i < count; i++)
+    {
+        var item = items[i];
+
+        if ([item action] === @selector(makeKeyAndOrderFront:) && [[item target] isKindOfClass:[CPWindow class]])
+            listedWindows.push([item target]);
+    }
+
+    return listedWindows;
+}
+
+/* @ignore */
+- (CPString)_windowsMenuTitle:(CPString)aString filename:(BOOL)isFilename
+{
+    if (!isFilename)
+        return aString;
+
+    var directory = [aString stringByDeletingLastPathComponent];
+
+    if (![directory length])
+        return [aString lastPathComponent];
+
+    return [aString lastPathComponent] + @" — " + directory;
+}
+
 /*!
     Opens the shared color panel.
     @param aSender

AppKit/CPWindow/CPWindow.j

@@ -177,6 +177,7 @@ var CPWindowActionMessageKeys = [
     BOOL                                _isVisible;
     BOOL                                _hasBeenOrderedIn @accessors;
     BOOL                                _isMiniaturized;
+    BOOL                                _isExcludedFromWindowsMenu;
     BOOL                                _isAnimating @accessors(setter=_setAnimating:);
     BOOL                                _hasShadow;
     BOOL                                _isMovableByWindowBackground;
@@ -1129,6 +1130,8 @@ CPTexturedBackgroundWindowMask
         if (CPDOMAvailable)
             [_platformWindow order:orderingMode window:self relativeTo:CPApp._windows[otherWindowNumber]];
     }
+
+    [self _updateWindowsMenuItem];
 }
 
 /*!
@@ -1792,6 +1795,54 @@ CPTexturedBackgroundWindowMask
 
     [_windowView setTitle:aTitle];
     [_platformWindow _setTitle:_title window:self];
+
+    [self _updateWindowsMenuItem];
+}
+
+/*!
+    Returns \c YES if the window is never listed in the application's Windows menu.
+*/
+- (BOOL)isExcludedFromWindowsMenu
+{
+    return _isExcludedFromWindowsMenu;
+}
+
+/*!
+    Sets whether the window is excluded from the application's Windows menu. By default, a window
+    is listed while it is visible or miniaturized, has a title and can become the main window.
+    @param shouldBeExcluded \c YES to exclude the window from the Windows menu
+*/
+- (void)setExcludedFromWindowsMenu:(BOOL)shouldBeExcluded
+{
+    if (_isExcludedFromWindowsMenu === shouldBeExcluded)
+        return;
+
+    _isExcludedFromWindowsMenu = shouldBeExcluded;
+
+    [self _updateWindowsMenuItem];
+}
+
+/* @ignore */
+- (void)_updateWindowsMenuItem
+{
+    if (_isExcludedFromWindowsMenu || ![_title length])
+        [self _removeFromWindowsMenu];
+
+    else if ([self canBecomeMainWindow])
+        [CPApp changeWindowsItem:self title:_title filename:NO];
+
+    // A miniaturized window stays listed, so that the user can bring it back from the menu.
+    else if (_isMiniaturized)
+        [CPApp updateWindowsItem:self];
+
+    else
+        [self _removeFromWindowsMenu];
+}
+
+/* @ignore */
+- (void)_removeFromWindowsMenu
+{
+    [CPApp removeWindowsItem:self];
 }
 
 /*!
@@ -2437,6 +2488,8 @@ CPTexturedBackgroundWindowMask
     [[CPNotificationCenter defaultCenter] postNotificationName:CPWindowDidMiniaturizeNotification object:self];
 
     _isMiniaturized = YES;
+
+    [self _updateWindowsMenuItem];
 }
 
 /*!
@@ -2449,6 +2502,8 @@ CPTexturedBackgroundWindowMask
     [[CPNotificationCenter defaultCenter] postNotificationName:CPWindowDidDeminiaturizeNotification object:self];
 
     _isMiniaturized = NO;
+
+    [self _updateWindowsMenuItem];
 }
 
 /*!
@@ -2549,6 +2604,9 @@ CPTexturedBackgroundWindowMask
     [self _orderOutRecursively:NO];
     [self _detachFromChildrenClosing:!_parentWindow];
 
+    // A closed window leaves the Windows menu, even if it was miniaturized.
+    [self _removeFromWindowsMenu];
+
     if (_releasedWhenClosed)
         [_contentView _releaseRecursively];
 }
@@ -2630,6 +2688,8 @@ CPTexturedBackgroundWindowMask
 
     [_windowView noteMainWindowStateChanged];
 
+    [CPApp updateWindowsItem:self];
+
     [[CPNotificationCenter defaultCenter]
         postNotificationName:CPWindowDidBecomeMainNotification
                       object:self];
@@ -2649,6 +2709,8 @@ CPTexturedBackgroundWindowMask
 
     _platformWindow._currentMainWindow = nil;
     [_windowView noteMainWindowStateChanged];
+
+    [CPApp updateWindowsItem:self];
 }
 
 - (void)_updateMainAndKeyWindows
@@ -2763,6 +2825,28 @@ CPTexturedBackgroundWindowMask
     [toolbar setVisible:![toolbar isVisible]];
 }
 
+/*!
+    Validates the user interface items whose actions the window handles.
+
+    For \c -toggleToolbarShown:, the item's title becomes "Hide Toolbar" or "Show Toolbar"
+    to reflect the toolbar's visibility, and the item is enabled only when the window has
+    a toolbar. Every other item is enabled.
+    @param anItem the item to validate
+    @return \c YES if the item should be enabled
+*/
+- (BOOL)validateUserInterfaceItem:(id /*<CPValidatedUserInterfaceItem>*/)anItem
+{
+    if ([anItem action] !== @selector(toggleToolbarShown:))
+        return YES;
+
+    var toolbar = [self toolbar];
+
+    if ([anItem respondsToSelector:@selector(setTitle:)])
+        [anItem setTitle:[toolbar isVisible] ? @"Hide Toolbar" : @"Show Toolbar"];
+
+    return !!toolbar;
+}
+
 - (void)_noteToolbarChanged
 {
     var frame = CGRectMakeCopy([self frame]),

Tests/AppKit/CPApplicationWindowsMenuTest.j

@@ -0,0 +1,172 @@
+@import <AppKit/CPApplication.j>
+@import <AppKit/CPMenu.j>
+@import <AppKit/CPPanel.j>
+@import <AppKit/CPWindow.j>
+
+@implementation CPApplicationWindowsMenuTest : OJTestCase
+{
+    CPApplication   app;
+    CPMenu          windowsMenu;
+}
+
+- (void)setUp
+{
+    app = [[CPApplication alloc] init];
+
+    // A main menu whose Window submenu carries the name nib2cib gives the Windows menu.
+    var mainMenu = [[CPMenu alloc] initWithTitle:@"Main"],
+        windowsMenuItem = [[CPMenuItem alloc] initWithTitle:@"Window" action:nil keyEquivalent:@""];
+
+    windowsMenu = [[CPMenu alloc] initWithTitle:@"Window"];
+    [windowsMenu _setMenuName:@"_CPWindowsMenu"];
+    [windowsMenu addItemWithTitle:@"Minimize" action:@selector(performMiniaturize:) keyEquivalent:@"m"];
+
+    [windowsMenuItem setSubmenu:windowsMenu];
+    [mainMenu addItem:windowsMenuItem];
+    [app setMainMenu:mainMenu];
+}
+
+- (void)tearDown
+{
+    CPApp = nil;
+}
+
+- (CPWindow)visibleWindowWithTitle:(CPString)aTitle
+{
+    var aWindow = [[CPWindow alloc] initWithContentRect:CGRectMake(0.0, 0.0, 200.0, 100.0) styleMask:CPTitledWindowMask];
+
+    // There is no DOM in the test runner, so ordering a window in does not make it visible.
+    aWindow._isVisible = YES;
+    [aWindow setTitle:aTitle];
+
+    return aWindow;
+}
+
+- (void)testWindowsMenuIsFoundInMainMenu
+{
+    [self assert:windowsMenu equals:[app windowsMenu]];
+}
+
+- (void)testVisibleTitledWindowIsListedBelowSeparator
+{
+    var aWindow = [self visibleWindowWithTitle:@"Report"];
+
+    [self assert:3 equals:[windowsMenu numberOfItems]];
+    [self assertTrue:[[windowsMenu itemAtIndex:1] isSeparatorItem] message:@"a separator should precede the window list"];
+
+    var item = [windowsMenu itemAtIndex:2];
+
+    [self assert:@"Report" equals:[item title]];
+    [self assert:aWindow equals:[item target]];
+    [self assert:@selector(makeKeyAndOrderFront:) equals:[item action]];
+}
+
+- (void)testRetitlingWindowChangesItsItem
+{
+    var aWindow = [self visibleWindowWithTitle:@"Report"];
+
+    [aWindow setTitle:@"Final Report"];
+
+    [self assert:3 equals:[windowsMenu numberOfItems]];
+    [self assert:@"Final Report" equals:[[windowsMenu itemAtIndex:2] title]];
+}
+
+- (void)testUntitledWindowIsNotListed
+{
+    [self visibleWindowWithTitle:@""];
+
+    [self assert:1 equals:[windowsMenu numberOfItems]];
+}
+
+- (void)testExcludedWindowIsNotListed
+{
+    var aWindow = [self visibleWindowWithTitle:@"Report"];
+
+    [aWindow setExcludedFromWindowsMenu:YES];
+
+    [self assert:1 equals:[windowsMenu numberOfItems]];
+
+    [aWindow setExcludedFromWindowsMenu:NO];
+
+    [self assert:3 equals:[windowsMenu numberOfItems]];
+}
+
+- (void)testPanelIsNotListed
+{
+    var panel = [[CPPanel alloc] initWithContentRect:CGRectMake(0.0, 0.0, 200.0, 100.0) styleMask:CPTitledWindowMask];
+
+    panel._isVisible = YES;
+    [panel setTitle:@"Inspector"];
+
+    [self assert:1 equals:[windowsMenu numberOfItems]];
+}
+
+- (void)testMainWindowIsChecked
+{
+    var firstWindow = [self visibleWindowWithTitle:@"First"],
+        secondWindow = [self visibleWindowWithTitle:@"Second"];
+
+    [firstWindow makeMainWindow];
+
+    [self assert:CPOnState equals:[[windowsMenu itemAtIndex:2] state]];
+    [self assert:CPOffState equals:[[windowsMenu itemAtIndex:3] state]];
+
+    [secondWindow makeMainWindow];
+
+    [self assert:CPOffState equals:[[windowsMenu itemAtIndex:2] state]];
+    [self assert:CPOnState equals:[[windowsMenu itemAtIndex:3] state]];
+}
+
+- (void)testClosingLastWindowRemovesItemAndSeparator
+{
+    var aWindow = [self visibleWindowWithTitle:@"Report"];
+
+    [aWindow close];
+
+    [self assert:1 equals:[windowsMenu numberOfItems]];
+    [self assert:@"Minimize" equals:[[windowsMenu itemAtIndex:0] title]];
+}
+
+- (void)testClosingOneWindowKeepsSeparatorForOthers
+{
+    var firstWindow = [self visibleWindowWithTitle:@"First"];
+
+    [self visibleWindowWithTitle:@"Second"];
+    [firstWindow close];
+
+    [self assert:3 equals:[windowsMenu numberOfItems]];
+    [self assert:@"Second" equals:[[windowsMenu itemAtIndex:2] title]];
+}
+
+- (void)testFilenameItemShowsNameThenDirectory
+{
+    var aWindow = [[CPWindow alloc] initWithContentRect:CGRectMake(0.0, 0.0, 200.0, 100.0) styleMask:CPTitledWindowMask];
+
+    [app addWindowsItem:aWindow title:@"/Users/me/Report.txt" filename:YES];
+
+    [self assert:@"Report.txt — /Users/me" equals:[[windowsMenu itemAtIndex:2] title]];
+}
+
+- (void)testAddingListedWindowAgainDoesNothing
+{
+    var aWindow = [self visibleWindowWithTitle:@"Report"];
+
+    [app addWindowsItem:aWindow title:@"Other" filename:NO];
+
+    [self assert:3 equals:[windowsMenu numberOfItems]];
+    [self assert:@"Report" equals:[[windowsMenu itemAtIndex:2] title]];
+}
+
+- (void)testSetWindowsMenuMovesWindowItems
+{
+    var aWindow = [self visibleWindowWithTitle:@"Report"],
+        otherMenu = [[CPMenu alloc] initWithTitle:@"Windows"];
+
+    [app setWindowsMenu:otherMenu];
+
+    [self assert:1 equals:[windowsMenu numberOfItems]];
+    [self assert:1 equals:[otherMenu numberOfItems]];
+    [self assert:aWindow equals:[[otherMenu itemAtIndex:0] target]];
+}
+
+@end

Tests/Manual/TextEditor/WORKLIST.md

@@ -6,9 +6,7 @@ TextEditor is a document-based test application and a tutorial. When the app use
 
 ### Windows and documents
 
-- **The Window menu does not list document windows.** It has no items to show or change the active window. AppKit has no equivalent of the Cocoa windows menu support in `NSApplication`: `windowsMenu`, `addWindowsItem:title:filename:`, `changeWindowsItem:title:filename:`, `removeWindowsItem:` and `arrangeInFront:`. nib2cib marks the menu as `_CPWindowsMenu` (`Tools/nib2cib/NSMenu.j:31`), but nothing in AppKit reads that name.
 - **AppKit ignores `CPBundleDisplayName`.** Cocoa uses `CFBundleDisplayName` for the name that the user sees, and `CFBundleName` as the short name. AppKit reads only `CPBundleName`: for the About panel title (`CPApplication.j:389`) and for the browser tab title (`_CPMenuBarWindow.j:106`). Both should use `CPBundleDisplayName`, with `CPBundleName` as the fallback.
-- **The Show Toolbar menu item does not show the toolbar state.** `-[CPWindow toggleToolbarShown:]` works (`CPWindow.j:2758`), but `CPWindow` has no `validateMenuItem:`. In Cocoa, `NSWindow` validates this item: it changes the title between "Show Toolbar" and "Hide Toolbar", and disables the item when the window has no toolbar. `CPMenu` already asks the target through `validateMenuItem:` (`CPMenu.j:699`), so the fix is a `validateMenuItem:` in `CPWindow` that handles `toggleToolbarShown:` and returns `YES` for every other action.
 - **The "save changes" alert is modal for the whole app.** `CPDocument` shows it with `runModal` (`CPDocument.j:903`). Cocoa attaches it as a sheet to the document window, so that only that document is blocked.
 - **Save cannot work without a server.** With no file URL, Save uses `CPSavePanel`, which falls back to `window.prompt` (`CPSavePanel.j:79`). `CPDocument` then sends an HTTP POST. A storage facade is needed, with local (IndexedDB) and remote (HTTP) implementations.
 
@@ -70,4 +68,6 @@ TextEditor is a document-based test application and a tutorial. When the app use
 - Paragraph indents through the ruler markers.
 - The "save changes" alert when a document closes.
 - A minimum size for document windows, through the window delegate. The window stops at the minimum while the pointer keeps moving, as in macOS, and it can still be moved and resized afterwards.
+- View > Show Toolbar and Hide Toolbar: the item's title follows the toolbar's visibility.
+- The Window menu lists the open document windows, marks the main window with a check mark and brings a window to the front when chosen. Bring All to Front can be tested once documents open in separate browser windows.
 - Builds with both the legacy toolchain and capp-build.