(In reply to Magnus Melin [:mkmelin] from comment #16) > ::: calendar/base/content/calendar-chrome-startup.js > @@ +216,5 @@ > > + * > > + * @param {boolean} [remove] Whether to remove event listeners instead of adding them. > > + */ > > +function setUpCalendarAppMenuItems(remove) { > > + const addOrRemoveListener = remove ? "removeEventListener" : "addEventListener"; > > I think it's preferable not to parametrize this. It's not super readable. > > On the other hand, I'm not sure why we bother with removing the listeners > ever either. The go away with the element anyway. Ah, good point! I've removed the code that removes the listeners for these menu items, and getting rid of the parameterization. I also made a part4 patch that does the same with the code for tearing down the appmenu buttons.
Bug 1558599 Comment 18 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Magnus Melin [:mkmelin] from comment #16) > ::: calendar/base/content/calendar-chrome-startup.js > @@ +216,5 @@ > > * > > * @param {boolean} [remove] Whether to remove event listeners instead of adding them. > > */ > > function setUpCalendarAppMenuItems(remove) { > > const addOrRemoveListener = remove ? "removeEventListener" : "addEventListener"; > > I think it's preferable not to parametrize this. It's not super readable. > > On the other hand, I'm not sure why we bother with removing the listeners > ever either. The go away with the element anyway. Ah, good point! I've removed the code that removes the listeners for these menu items, and getting rid of the parameterization. I also made a part4 patch that does the same with the code for tearing down the appmenu buttons.
(In reply to Magnus Melin [:mkmelin] from comment #16) > ::: calendar/base/content/calendar-chrome-startup.js > @@ +216,5 @@ > > function setUpCalendarAppMenuItems(remove) { > > const addOrRemoveListener = remove ? "removeEventListener" : "addEventListener"; > > I think it's preferable not to parametrize this. It's not super readable. > > On the other hand, I'm not sure why we bother with removing the listeners > ever either. The go away with the element anyway. Ah, good point! I've removed the code that removes the listeners for these menu items, and getting rid of the parameterization. I also made a part4 patch that does the same with the code for tearing down the appmenu buttons.