Max-Github / FireFoxInvertColors

A simple addon that will invert a web page colors.
GNU General Public License v3.0
56 stars 15 forks source link

New feature: per-tab invert #16

Closed iam-TJ closed 6 years ago

iam-TJ commented 6 years ago

Repurposes the tab's context menu "Invert Colors" menu entry to only affect the current tab.

Requires the additional 'sessions' permission in order to use browser.sessions.{get,set}TabValue() for storing per-tab state.

Closes #9.

Signed-of-by: Tj hacker@iam.tj

iam-TJ commented 6 years ago

Please don't merge this yet, I've found an inconsistency when a single tab is un-inverted and then options.html changes are made and then the same tab is re-inverted. Images remain inverted.

iam-TJ commented 6 years ago

Tests now seem to be passing consistently, OK to merge.

Max-Github commented 6 years ago

Thinking about this a little more, I am not sure this is a needed feature at this point. Most users are only going to be viewing a single tab at a time anyways, and they can easily switch between the "Invert" states.

iam-TJ commented 6 years ago

The reason for the patch is to reduce the significant load on the browser when there are multiple tabs open. It slows down page load/redraw -> completed times dramatically.

In my case, and several other colleagues/friends who are using the extension, we have from 30 to 100 tabs (or more!) and so each time the extension is activated/deactivated there is a significant delay and spike in CPU usage as the changes are applied that is enough to interfere with the user experience.

With this patch the impact can be limited to the active tab - whether it is currently inverted or not.

It also makes sense that the per-tab context menu option only affects the tab it is invoked on (That's the entire point of /context/ aware menus.

Max-Github commented 6 years ago

That is a very good point that I did not consider. This will be tested and merged in this weekend.