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 @@
> >  *
> >  * @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.

Back to Bug 1558599 Comment 18