jingyu9575 / tabs-to-bookmark-groups

(Work in progress) Firefox extension to save the tabs to a bookmark group and switch between different groups.
https://addons.mozilla.org/firefox/addon/tabs-to-bookmark-groups/
Mozilla Public License 2.0
4 stars 1 forks source link

Request: Keep favicons at unloaded tabs #11

Closed EstherMoellman closed 4 years ago

EstherMoellman commented 4 years ago

@jingyu9575 , I'm not sure this is possible, but if doable, please I request the following feature: To keep favicons at unloaded tabs.

Currently, when a group is changed, tabs are unloaded, and only their labels appears. It will be nice to have also faicons at unloaded tabs.

Thank you!

jingyu9575 commented 4 years ago

The problems:

  1. Extensions cannot create discarded tabs with favicons. See Firefox Bug 1475240.
  2. Extensions cannot access bookmark favicons. See Firefox Bug 1315616.

Possible workarounds:

  1. Create placeholder tabs that automatically redirect to the original tab. The placeholder tab must be created as a non-discarded tab, to set the icon and discard itself. It may decrease performance.
    • This is similar to Tab Session Manager's workaround in Chrome where discarded tabs are not supported.
  2. When saving the group, save the icons as well. However, icons must go to extension storage, because bookmark icon cannot be accessed. I think you've experienced extension storage corruption and that's one of the reason you want bookmarks.

What do you think?

EstherMoellman commented 4 years ago

... hummm, both alternatives don't seem the best.

From my ignorance, and correct me if I'm wrong, the second option means a potential risk for future conflicts with Firefox bugs or functions etc. I'm not saying this is existing right now, but I'm saying that keeping bookmarks at FF' bookmark editor, may have less potential issues compared to keep bookmarks at add-on storage. Seems to me that as long as your add-on uses more and more FF' stuff (i.e. FF' bookmark editor VS add-on own storage), it may have less and less probabilities of issues with FF.

If I'm right, then the first option may be the right alternative. But I have no idea about the negative performance decrease. is it big? Medium? Minimum? If it is minimum, I can totally live with it! For example, by using "hiding/discarding" when you open a group of 100 unloaded tabs, STG add-on can do that almost instantaneously. But your add-on takes few milliseconds to do that. And I don't care! The performance impact is so minimum, that I don't care! So I leave you the final word about this first first option (emulating TabSessionManager' workaround at Chrome).

I really don't have enough knowledge to choose which alternative will be the best for keeping flavicons at unloaded tabs. And I totally trust you... you're the right person to decide if it is worth doing, and how to do it. Take all the time you need, don't rush, the current TTBG is almost perfect, so dedicate yourself to all these new requests, only when you want and if you want.

Big hug! :)

PS: I sent an email to Martin Brinkmann (Ghacks.net) introducing your TTBG. I hope he will be interested on writing a review of your fantastic add-on.

jingyu9575 commented 4 years ago

It is not a choice; there are 2 Firefox bugs, and both workarounds need to be implemented for the feature. Maybe I'll try it and see if the workarounds are acceptable.

jingyu9575 commented 4 years ago

I've added an option to keep favicons, in extension storage. Try the CI build and see if the performance is acceptable.

jingyu9575 commented 4 years ago

When a group is changed, all tabs seem to be loaded for a 1/2 second When a tab is selected, it has a kind of "grey flash" Originally posted by @EstherMoellman in https://github.com/jingyu9575/tabs-to-bookmark-groups/issues/14#issuecomment-583012030

As I have said, extensions cannot create discarded tabs with favicons, so the workaround is to create a internal tab, set the icon, then discard it.

EstherMoellman commented 4 years ago

As I have said, extensions cannot create discarded tabs with favicons, so the workaround is to create a internal tab, set the icon, then discard it.

Yeap, you're totally right, you said that. But sometimes one thing is to read and another is to see, and now by seeing, I understand what you wrote before. And your solution works, favicons are there, but personally I think is too heavy those two collateral effects (loaded for 1/2 second before unloaded + grey flash / page on background).

As I said, I don't need favicons. So, I already disabled "load icons", and everything works beautiful for me. So Genius, now is up to you: 1) You can close the issue, offering the existing option for favicons, and those like me that don't like that, they can just disable favicons. or 2) You can try another alternative without "collateral effects" (if it exists). As you said, some stuff is going to be a trade off, perhaps favicons without collateral effects is not doable.