darwinye / tab-organizer

Automatically exported from code.google.com/p/tab-organizer
Other
0 stars 0 forks source link

Queue selection code is ... hacky and potentially broken. #101

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I just took a look at how we handle tab selection. It seems simple enough... 
each window has a "queue" property which is an array. Every item in the array 
is an element that has been selected with Ctrl, Shift, etc.

This obviously works, and appears to be okay, but... when we remove or update 
or otherwise change a tab, we do not update the "queue" array. This means that 
the following is possible:

1) Select a single tab in a window.
2) Close that tab.
3) Use Menu -> Move selected to... -> New Window.

The problem is, that even though the tab has been removed, there's still a 
reference to it in the "queue" array. I used a hacky fix by removing the tab 
from the queue by using the "DOMNodeRemovedFromDocument" event, but it really 
does feel hacky to me.

We could go in and manually clean up the queue every time a tab is removed or 
updated, but in addition to potentially being a lot of work, it's possible I 
might miss an obscure edge case.

What would be better is to rethink the whole "queue" array concept. Perhaps we 
should just use `container.tabList.querySelectorAll(".tab[data-selected]")`, 
which would ensure that the results are correct.

The obvious downside being performance: rather than having a cost when 
adding/removing elements to the array, we have a cost every time we need to do 
anything to the array, including look up the array's length!

Another idea is to embrace things like "DOMNodeRemovedFromDocument", but I'm 
not convinced that's the best way.

Original issue reported on code.google.com by pcxunlimited@gmail.com on 17 Nov 2010 at 12:56

GoogleCodeExporter commented 9 years ago

Original comment by pcxunlimited@gmail.com on 21 Nov 2010 at 6:52

GoogleCodeExporter commented 9 years ago

Original comment by pcxunlimited@gmail.com on 30 Apr 2012 at 11:31

GoogleCodeExporter commented 9 years ago

Original comment by pcxunlimited@gmail.com on 27 Aug 2012 at 9:51

GoogleCodeExporter commented 9 years ago

Original comment by pcxunlimited@gmail.com on 27 Aug 2012 at 11:03

GoogleCodeExporter commented 9 years ago

Original comment by pcxunlimited@gmail.com on 27 Aug 2012 at 11:15

GoogleCodeExporter commented 9 years ago

Original comment by pcxunlimited@gmail.com on 27 Aug 2012 at 11:17

GoogleCodeExporter commented 9 years ago

Original comment by pcxunlimited@gmail.com on 27 Aug 2012 at 11:39

GoogleCodeExporter commented 9 years ago

Original comment by pcxunlimited@gmail.com on 27 Aug 2012 at 11:47