search

Pauan / tab-organizer

Google Chrome Extension that makes it easier to manage many tabs!
MIT License
67 stars 8 forks source link

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

Open Pauan opened 9 years ago

Pauan commented 9 years ago

Originally reported on Google Code with ID 101

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.

Reported by pcxunlimited on 2010-11-17 00:56:40

Pauan commented 9 years ago

Reported by pcxunlimited on 2010-11-21 18:52:12

Pauan commented 9 years ago

Reported by pcxunlimited on 2012-04-30 23:31:25

Pauan commented 9 years ago

Reported by pcxunlimited on 2012-08-27 21:51:08

Pauan commented 9 years ago

Reported by pcxunlimited on 2012-08-27 23:03:36

Pauan commented 9 years ago

Reported by pcxunlimited on 2012-08-27 23:15:27

Pauan commented 9 years ago

Reported by pcxunlimited on 2012-08-27 23:17:41

Pauan commented 9 years ago

Reported by pcxunlimited on 2012-08-27 23:39:18

Pauan commented 9 years ago

Reported by pcxunlimited on 2012-08-27 23:47:28