Open tabs in background by default and make it configurable #1744
Conversation
This change restores the behaviour of opening new tabs in the background by default. This functionality had been changed when the new tabs bar was implemented. The reason was that opening a tab in the background leaves it in an uninitialized state until the user clicks on that tab, and this made it difficult to keep the tabs bar updated in real time. We will use the domain name as the title of background tabs until the user clicks on them (this was empty previously).
Add a new switch to Options / Display that allows the user to choose whether new tabs will be activated immediately or they will remain in the background. By default, new tabs will be open in the background. For now, this setting only affects the contextual menu.
| @@ -192,6 +192,10 @@ private Session addSession(@NonNull Session aSession) { | |||
| mStoreSubscription.resume(); | |||
| } | |||
|
|
|||
| for (SessionChangeListener listener : mSessionChangeListeners) { | |||
| listener.onSessionAdded(aSession); | |||
| } | |||
There was a problem hiding this comment.
Hmm, I am a bit worried about this because this might potentially add side effects as we were not doing this before. The Session class is the one calling those methods, I think we should be able to do something without having to add this
There was a problem hiding this comment.
This might be a bit confusing because SessionStore also has its own onSessionAdded() callback which gets called by the Session objects themselves. However, that is a more delicate method with plenty of side effects.
Basically, the problem is that SessionStore.onSessionAdded() makes this very fragile call:
ComponentsAdapter.get().addSession(aSession);This method updates Gecko's internal data. It can not be called twice for the same Session because Gecko might crash with "Tab with same ID already exists". I might also crash when a session has not been fully initialized before being added.
On the other hand, going back to this specific change, I think that it is not dangerous at all because the only listener for this callback is the tabs UI. That UI has to be updated after the new tab has been added to the list of tabs (mSessions).
This change restores the behaviour of opening new tabs in the background by default.
This functionality had been changed when the new tabs bar was implemented. The reason was that opening a tab in the background leaves it in an uninitialized state until the user clicks on that tab, and this made it difficult to keep the tabs bar
updated in real time.
Additionally, we will use the domain name as the title of background tabs until the user clicks on them (this was empty previously).
This change also adds a new switch to Options / Display that allows the user to choose whether new tabs will be activated immediately or they will remain in the background.
For now, this setting only affects the contextual menu.