465281 - Inform the user when a new tab is opened

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement)

Comment 1

[Christian Sejersen]

If a site opens popups (which are displayed as tabs) the user needs to be informed

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

we do support the "TabOpen" event so add-ons or other code could watch for it and display some UI.

Comment 3

[Christian Sejersen]

You don't think a notification would be useful?

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #3)
> You don't think a notification would be useful?

Maybe. Not a notification bar across the top of the page, but maybe something else. Just not sure of the priority. This would only be for tabs loaded in the background.

We could slide out the tab strip to show the new tab and slide it back quickly.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #4)
> We could slide out the tab strip to show the new tab and slide it back quickly.

I think this would be the way to go.  Or maybe fade-in/fade-out.  I worry about it being distracting to the user though, and I think a notification bar would be too much.

Comment 6

[Christian Sejersen]

I think (comment #3) sounds like a good idea, was just worried that comment #2 implied to leave it up to add-ons.

Comment 7

[Madhava Enros [:madhava]]

I don't think we should pan the sidebar in and out.  Maybe thumbnails (even empty -- just an icon) that animatedly float over off the left-hand-side of the screen.  Where you only ever see a bit of them.

Comment 8

[Madhava Enros [:madhava]]

Attachment: indicators (like alerts) that show you where a tab has been opened in the background

This is what I'd like to see.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.1

Adding a NewTabPopup object - it's very similar to the BookmarkPopup one, I'm wondering if we should (in a near future) create a more abstract object for handling popup...

I'm not sure that decorate the new background tab is useful since it's always the last one.

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Add the css rule for wince theme

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 395805 [details] [diff] [review]
Add the css rule for wince theme

>diff -r 30e5022551d0 chrome/content/browser-ui.js
>     document.getElementById("tabs").addEventListener("TabSelect", this, true);
>+    window.addEventListener("TabOpen", this, true);

Let's attach to "tabs" like we do with "TabSelect". Make a local variable for tabs.

>+      case "TabOpen":
>+        if (!this.isTabsVisible())
>+          NewTabPopup.toggle(aEvent.target, true);
>+        break;

I don't think a toggle is what we want. What happens when 3 background tabs open at the same time? I think we just want to keep the popup open but reset the "hide" timeout, which is kinda what NewTabPopup.show() does

>+var NewTabPopup = {
>+  _newtabPopupTimeout: -1,
>+
>+  get box() {
>+    return document.getElementById("newtab-popup");
>+  },

Make this a smart getter? One that deletes itself and just returns the element;

>+  show: function(aTab, aAutoClose) {
>+    this._tab = aTab;
>+    this.box.top = aTab.boxObject.y + (aTab.boxObject.height / 3);

I think Stuart would like us to move away form boxObject and use getBoundingClientRect

I think we need to deal with the this.box.left too


>+    if (aAutoClose) {

Can we check for an existing timeout in effect and cancel it here too?

>+  toggle: function(aTab, aAutoClose) {
>+    if (this.box.hidden)
>+      this.show(aTab, aAutoClose);
>+    else
>+      this.hide();
>+  }

Not sure we need this method


>+    <vbox id="newtab-popup" hidden="true" class="panel-dark" onclick="NewTabPopup.selectTab()" left="21">
>+      <label value="&newtab.opened;"/>
>+    </vbox>

No need for the left="21" right? left="0" top="0" is good enough since you adjust them

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

Thanks for the review, this patch address the above comments.

A few point though.

> I think we need to deal with the this.box.left too

Don't we want to show the popup -only- when the tabs sidebar is hidden? If so we don't really need to play with left positioning, right?

> 
> >+    <vbox id="newtab-popup" hidden="true" class="panel-dark" onclick="NewTabPopup.selectTab()" left="21">
> >+      <label value="&newtab.opened;"/>
> >+    </vbox>
> 
> No need for the left="21" right? left="0" top="0" is good enough since you
> adjust them


See above.

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Screenshot: single tab

Comment 14

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Screenshot: multiple tab

Comment 15

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.3

The patch is using PluralForm.jsm now

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed: https://hg.mozilla.org/mobile-browser/rev/4dd9059d0448

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

Verified with 20091001 1.9.2 beta4 candidate on my n810