Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

DISPOSE of MENU item not functioning as expected. #16508

Closed
1 task done
ghost opened this issue Jan 7, 2018 · 3 comments
Closed
1 task done

DISPOSE of MENU item not functioning as expected. #16508

ghost opened this issue Jan 7, 2018 · 3 comments

Comments

@ghost
Copy link

ghost commented Jan 7, 2018

Prerequisites

  • Put an X between the brackets on this line if you have done all of the following:
    • Reproduced the problem in Safe Mode.
    • Followed all applicable steps in the debugging guide.
    • Checked the FAQs on the message board for common solutions.
    • Checked that your issue isn't already filed.
    • Checked that there is not already an Atom package that provides the described functionality.

Description

Tags: API, menu, atom.menu.add(), dispose()

If a menu object is disposed, this object is not disposed properly when one of the object's items is a 'separator`.

Building the object would be with the instruction
favoritesMenu = atom.menu.add( .. )

Disposing of the object would be..
favoritesMenu.dispose()


Steps to Reproduce

  1. Open the development console in Atom.
  2. Execute code in the console to create a complex menu entry.
    favoritesMenu = atom.menu.add( .. )
  3. Execute code in the console to dispose of the menu entry.
    favoritesMenu.dispose()

Scenario1 - Does work

Execute code in the development console...

  • Creates menu items as expected.

    Notice the - - - - item.
favoritesMenu = atom.menu.add ([{
  label: 'Extra',
  submenu : [{
    label: 'Test',
    submenu: [
      {label: 'item1'},
      {label: '- - - - '},
      {label: 'item2'}
    ]
  }]
}])
  • Dispose all menu items as expected.
favoritesMenu.dispose()

Scenario2 - Does not work

Execute code in the development console...

  • Creates menu items as expected.

    Notice the separator item.
favoritesMenu = atom.menu.add ([{
  label: 'Extra',
  submenu : [{
    label: 'Test',
    submenu: [
      {label: 'item1'},
      {type: 'separator'},
      {label: 'item2'}
    ]
  }]
}])
  • Attempt to dispose all menu items with unexpected result.
favoritesMenu.dispose()

Expected behavior: [What you expect to happen]
All items are removed from the menu if dispose() is executed.

Actual behavior: [What actually happens]
Scenario1 occurs as expected. Scenario2 does not work as expected.
Scenario2: Items item1 and item2 is removed as expected. however Extra -> Test -> separator remains. Restarting Atom brings back normality.

Reproduces how often: [What percentage of the time does it reproduce?]
100%

Versions

  • Atom V1.23.2
  • Atom V1.24-beta 2
  • Windows 10 Professional

Additional Information

Originally reported by @gliviu on https://discuss.atom.io/t/menu-refresh-fails-for-separators/51255. @gliviu constructed similar coding in init.js which led him to discover the behaviour.

Thanks for your attention.
- Dan Padric

@50Wliu
Copy link
Contributor

50Wliu commented Jan 8, 2018

Confirmed.

@daviwil
Copy link
Contributor

daviwil commented Jan 24, 2019

Confirmed that this still occurs in Atom 1.34.0. It appears that these lines are the culprit:

https://github.com/atom/atom/blob/master/src/menu-helpers.js#L53-L55

When "unmerging" menu items that contain separators, those separators cannot be uniquely identified so they are just left hanging around in the menu after the other textual items have been removed. When the only items left are just separators, the parent menu does not get fully destroyed.

It appears that we need to refactor the menu management code a bit so that a uniqueness key can be added to separator items so that they can be identified when a group of menu items is being removed.

@darangi
Copy link
Contributor

darangi commented Sep 23, 2021

This has already been fixed in the latest version of atom.

@darangi darangi closed this as completed Sep 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants