From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Alan Third Newsgroups: gmane.emacs.bugs Subject: bug#44333: 27.1; macOS menu bar 2-clicks Date: Wed, 23 Dec 2020 20:33:57 +0000 Message-ID: References: <2B2932BB-DFD0-409C-9351-FACEC46927BB@acm.org> <20201031150101.GN59267@breton.holly.idiocy.org> <6F053CD0-A164-490F-9D15-225D8782C633@acm.org> <20201101172843.GQ59267@breton.holly.idiocy.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="/Bw1Hg923KSM+mBz" Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="15571"; mail-complaints-to="usenet@ciao.gmane.io" To: Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= , 44333@debbugs.gnu.org, Viktor Kharitonovich Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Dec 23 21:35:20 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ksAqt-0003pH-Hf for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 23 Dec 2020 21:35:19 +0100 Original-Received: from localhost ([::1]:38108 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ksAqo-00068l-DV for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 23 Dec 2020 15:35:16 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:59738) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ksAqc-00068b-3b for bug-gnu-emacs@gnu.org; Wed, 23 Dec 2020 15:35:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:42676) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ksAqb-0002Et-Sq for bug-gnu-emacs@gnu.org; Wed, 23 Dec 2020 15:35:01 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ksAqb-0000ky-QY for bug-gnu-emacs@gnu.org; Wed, 23 Dec 2020 15:35:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Alan Third Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 23 Dec 2020 20:35:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 44333 X-GNU-PR-Package: emacs Original-Received: via spool by 44333-submit@debbugs.gnu.org id=B44333.16087556522845 (code B ref 44333); Wed, 23 Dec 2020 20:35:01 +0000 Original-Received: (at 44333) by debbugs.gnu.org; 23 Dec 2020 20:34:12 +0000 Original-Received: from localhost ([127.0.0.1]:54222 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ksApn-0000jo-Ht for submit@debbugs.gnu.org; Wed, 23 Dec 2020 15:34:12 -0500 Original-Received: from outbound.soverin.net ([116.202.65.218]:55231) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ksApk-0000jZ-MU for 44333@debbugs.gnu.org; Wed, 23 Dec 2020 15:34:10 -0500 Original-Received: from smtp.soverin.net (unknown [10.10.3.24]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by outbound.soverin.net (Postfix) with ESMTPS id 586FC60157; Wed, 23 Dec 2020 20:34:02 +0000 (UTC) Original-Received: from smtp.soverin.net (smtp.soverin.net [159.69.232.138]) by soverin.net DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=idiocy.org; s=soverin; t=1608755641; bh=f7o8o75eTBsBwUKFDfB22bm9lupxGeaYc/SRmaQGpFs=; h=Date:From:To:Subject:References:In-Reply-To:From; b=L7cCfXqlkuaZG3QOBe8mKT3aD0isIibbNp7CQr9hNwQO3sdbpON914a769w2KObnY QA/sE2+aeoLrozh8tUHUQLa2wRtlFeSjvy95mGUAubRlrWw8elYEHWJPyqwJqShRcn +VH6CmUmDn5Qzi1cTD7q6setsJcijnR78RpgJMvlSxUj2ux5nbPiJ1ftz2eICB9qy1 goFTs/UZcWmd/CD4RVnks3pjreXb/FkAlQudS/WXw3i2XQlfFZouelWCCgymgDiHlL vyYV2U+19IdD3YBPa/jZx9jFyGiZ7YzzYAzZcJ1hwgTQTVG+8JpOYBv4wLlcFY9D0v WfLjZj+XN8fCQ== Original-Received: by breton.holly.idiocy.org (Postfix, from userid 501) id 5B96E202917995; Wed, 23 Dec 2020 20:33:57 +0000 (GMT) Mail-Followup-To: Alan Third , Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= , 44333@debbugs.gnu.org, Viktor Kharitonovich Content-Disposition: inline In-Reply-To: <20201101172843.GQ59267@breton.holly.idiocy.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:196638 Archived-At: --/Bw1Hg923KSM+mBz Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Sun, Nov 01, 2020 at 05:28:43PM +0000, Alan Third wrote: > On Sun, Nov 01, 2020 at 11:50:32AM +0100, Mattias Engdegård wrote: > > > If anyone else has a simpler solution I'd love to hear it. > > > > Maybe we are overthinking it. I don't think the Cocoa event loop > > really is running in a different thread. Anecdotal evidence > > (printf!) indicates that it isn't. > > On the Mac port? It certainly isn't on the NS port... I don't think > the Mac port even really uses the Cocoa event loop, although I could > easily be wrong. > > > (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=32864#38 has a > > previous investigation into the matter.) > > > > I just did the simplest thing possible and it works (with a slight > > latency at times, but perhaps tolerable). I'd like to know what's > > wrong with running Lisp in the event loop. It does appear safe to > > me. > > We used to have a *lot* of crashes that were related to the event loop > and lisp and so on, but if you're confident it isn't a problem feel > free to propose a patch and we can give it a go on master. OK, I've had a go at this and tried removing EVERYTHING related to the delayed menu stuff and, as you say, it seems fine, but I barely ever use the menus, so I could be missing something. For the record, the menu code (or maybe it's the toolbar, but I suspect the menus) kills GNUstep builds stone dead as soon as you try typing anything. It looks from the debugger like Emacs is still running, but it just won't update the screen. Turning off the menus appears to fix it. -- Alan Third --/Bw1Hg923KSM+mBz Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0001-Remove-NS-menu-synthesized-events-bug-44333.patch" >From 0e07430da3ad68bdc375aa1ac2688c2eadbbe88f Mon Sep 17 00:00:00 2001 From: Alan Third Date: Wed, 23 Dec 2020 20:12:02 +0000 Subject: [PATCH] Remove NS menu synthesized events (bug#44333) * src/nsmenu.m (ns_update_menubar): Remove GNUstep special case that's no longer needed. (ns_activate_menubar): ([EmacsMenu trackingNotification:]): ([EmacsMenu menuWillOpen:]): ([EmacsMenu menuDidClose:]): Remove unused functions. ([EmacsMenu menuNeedsUpdate:]): Remove menu tracking code. * src/nsterm.m (ns_check_menu_open): (ns_check_pending_open_menu): (ns_create_terminal): Remove unused functions. (ns_term_init): Get rid of menu tracking. --- src/nsmenu.m | 78 --------------------------------------- src/nsterm.h | 2 - src/nsterm.m | 101 --------------------------------------------------- 3 files changed, 181 deletions(-) diff --git a/src/nsmenu.m b/src/nsmenu.m index efad978316..d72f9c61be 100644 --- a/src/nsmenu.m +++ b/src/nsmenu.m @@ -58,10 +58,6 @@ /* Nonzero means a menu is currently active. */ static int popup_activated_flag; -/* Nonzero means we are tracking and updating menus. */ -static int trackingMenu; - - /* NOTE: toolbar implementation is at end, following complete menu implementation. */ @@ -141,10 +137,6 @@ t = -(1000*tb.time+tb.millitm); #endif -#ifdef NS_IMPL_GNUSTEP - deep_p = 1; /* until GNUstep NSMenu implements the Panther delegation model */ -#endif - if (deep_p) { /* Fully parse one or more of the submenus. */ @@ -463,17 +455,6 @@ ns_update_menubar (f, deep_p, nil); } -void -ns_activate_menubar (struct frame *f) -{ -#ifdef NS_IMPL_COCOA - ns_update_menubar (f, true, nil); - ns_check_pending_open_menu (); -#endif -} - - - /* ========================================================================== @@ -514,43 +495,6 @@ - (void)setFrame: (struct frame *)f frame = f; } -#ifdef NS_IMPL_COCOA --(void)trackingNotification:(NSNotification *)notification -{ - /* Update menu in menuNeedsUpdate only while tracking menus. */ - trackingMenu = ([notification name] == NSMenuDidBeginTrackingNotification - ? 1 : 0); - if (! trackingMenu) ns_check_menu_open (nil); -} - -- (void)menuWillOpen:(NSMenu *)menu -{ - ++trackingMenu; - -#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070 - // On 10.6 we get repeated calls, only the one for NSSystemDefined is "real". - if ( -#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 - NSAppKitVersionNumber < NSAppKitVersionNumber10_7 && -#endif - [[NSApp currentEvent] type] != NSEventTypeSystemDefined) - return; -#endif - - /* When dragging from one menu to another, we get willOpen followed by didClose, - i.e. trackingMenu == 3 in willOpen and then 2 after didClose. - We have updated all menus, so avoid doing it when trackingMenu == 3. */ - if (trackingMenu == 2) - ns_check_menu_open (menu); -} - -- (void)menuDidClose:(NSMenu *)menu -{ - --trackingMenu; -} - -#endif /* NS_IMPL_COCOA */ - /* Delegate method called when a submenu is being opened: run a 'deep' call to set_frame_menubar. */ - (void)menuNeedsUpdate: (NSMenu *)menu @@ -558,29 +502,7 @@ - (void)menuNeedsUpdate: (NSMenu *)menu if (!FRAME_LIVE_P (frame)) return; - /* Cocoa/Carbon will request update on every keystroke - via IsMenuKeyEvent -> CheckMenusForKeyEvent. These are not needed - since key equivalents are handled through emacs. - On Leopard, even keystroke events generate SystemDefined event. - Third-party applications that enhance mouse / trackpad - interaction, or also VNC/Remote Desktop will send events - of type AppDefined rather than SysDefined. - Menus will fail to show up if they haven't been initialized. - AppDefined events may lack timing data. - - Thus, we rely on the didBeginTrackingNotification notification - as above to indicate the need for updates. - From 10.6 on, we could also use -[NSMenu propertiesToUpdate]: In the - key press case, NSMenuPropertyItemImage (e.g.) won't be set. - */ - if (trackingMenu == 0) - return; -/*fprintf (stderr, "Updating menu '%s'\n", [[self title] UTF8String]); NSLog (@"%@\n", event); */ -#ifdef NS_IMPL_GNUSTEP - /* Don't know how to do this for anything other than Mac OS X 10.5 and later. - This is wrong, as it might run Lisp code in the event loop. */ ns_update_menubar (frame, true, self); -#endif } diff --git a/src/nsterm.h b/src/nsterm.h index 94472ec107..0b7e27c00f 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -1130,8 +1130,6 @@ ns_query_color (void *col, Emacs_Color *color_def, bool setPixel); extern NSColor *ns_lookup_indexed_color (unsigned long idx, struct frame *f); extern unsigned long ns_index_color (NSColor *color, struct frame *f); extern const char *ns_get_pending_menu_title (void); -extern void ns_check_menu_open (NSMenu *menu); -extern void ns_check_pending_open_menu (void); #endif /* Implemented in nsfns, published in nsterm. */ diff --git a/src/nsterm.m b/src/nsterm.m index 2a117a0780..161677484f 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -310,24 +310,6 @@ - (NSColor *)colorUsingDefaultColorSpace NULL, 0, 0 }; -#ifdef NS_IMPL_COCOA -/* - * State for pending menu activation: - * MENU_NONE Normal state - * MENU_PENDING A menu has been clicked on, but has been canceled so we can - * run lisp to update the menu. - * MENU_OPENING Menu is up to date, and the click event is redone so the menu - * will open. - */ -#define MENU_NONE 0 -#define MENU_PENDING 1 -#define MENU_OPENING 2 -static int menu_will_open_state = MENU_NONE; - -/* Saved position for menu click. */ -static CGPoint menu_mouse_point; -#endif - /* Convert modifiers in a NeXTstep event to emacs style modifiers. */ #define NS_FUNCTION_KEY_MASK 0x800000 #define NSLeftControlKeyMask (0x000001 | NSEventModifierFlagControl) @@ -4607,79 +4589,6 @@ in certain situations (rapid incoming events). } #endif -/* GNUstep does not have cancelTracking. */ -#ifdef NS_IMPL_COCOA -/* Check if menu open should be canceled or continued as normal. */ -void -ns_check_menu_open (NSMenu *menu) -{ - /* Click in menu bar? */ - NSArray *a = [[NSApp mainMenu] itemArray]; - int i; - BOOL found = NO; - - if (menu == nil) // Menu tracking ended. - { - if (menu_will_open_state == MENU_OPENING) - menu_will_open_state = MENU_NONE; - return; - } - - for (i = 0; ! found && i < [a count]; i++) - found = menu == [[a objectAtIndex:i] submenu]; - if (found) - { - if (menu_will_open_state == MENU_NONE && emacs_event) - { - NSEvent *theEvent = [NSApp currentEvent]; - struct frame *emacsframe = SELECTED_FRAME (); - - /* On macOS, the following can cause an event loop when the - Spotlight for Help search field is populated. Avoid this by - not postponing mouse drag and non-user-generated mouse down - events (Bug#31371). */ - if (([theEvent type] == NSEventTypeLeftMouseDown) - && [theEvent eventNumber]) - { - [menu cancelTracking]; - menu_will_open_state = MENU_PENDING; - emacs_event->kind = MENU_BAR_ACTIVATE_EVENT; - EV_TRAILER (theEvent); - - CGEventRef ourEvent = CGEventCreate (NULL); - menu_mouse_point = CGEventGetLocation (ourEvent); - CFRelease (ourEvent); - } - } - else if (menu_will_open_state == MENU_OPENING) - { - menu_will_open_state = MENU_NONE; - } - } -} - -/* Redo saved menu click if state is MENU_PENDING. */ -void -ns_check_pending_open_menu () -{ - if (menu_will_open_state == MENU_PENDING) - { - CGEventSourceRef source - = CGEventSourceCreate (kCGEventSourceStateHIDSystemState); - - CGEventRef event = CGEventCreateMouseEvent (source, - kCGEventLeftMouseDown, - menu_mouse_point, - kCGMouseButtonLeft); - CGEventSetType (event, kCGEventLeftMouseDown); - CGEventPost (kCGHIDEventTap, event); - CFRelease (event); - CFRelease (source); - - menu_will_open_state = MENU_OPENING; - } -} -#endif /* NS_IMPL_COCOA */ static int ns_read_socket (struct terminal *terminal, struct input_event *hold_quit) @@ -5416,7 +5325,6 @@ static Lisp_Object ns_new_font (struct frame *f, Lisp_Object font_object, terminal->set_new_font_hook = ns_new_font; terminal->implicit_set_name_hook = ns_implicitly_set_name; terminal->menu_show_hook = ns_menu_show; - terminal->activate_menubar_hook = ns_activate_menubar; terminal->popup_dialog_hook = ns_popup_dialog; terminal->set_vertical_scroll_bar_hook = ns_set_vertical_scroll_bar; terminal->set_horizontal_scroll_bar_hook = ns_set_horizontal_scroll_bar; @@ -5661,15 +5569,6 @@ Needs to be here because ns_initialize_display_info () uses AppKit classes. [NSApp setServicesMenu: svcsMenu]; /* Needed at least on Cocoa, to get dock menu to show windows */ [NSApp setWindowsMenu: [[NSMenu alloc] init]]; - - [[NSNotificationCenter defaultCenter] - addObserver: mainMenu - selector: @selector (trackingNotification:) - name: NSMenuDidBeginTrackingNotification object: mainMenu]; - [[NSNotificationCenter defaultCenter] - addObserver: mainMenu - selector: @selector (trackingNotification:) - name: NSMenuDidEndTrackingNotification object: mainMenu]; } #endif /* macOS menu setup */ -- 2.29.2 --/Bw1Hg923KSM+mBz--