Closed Bug 465281 Opened 11 years ago Closed 10 years ago

Inform the user when a new tab is opened

Categories

(Firefox for Android Graveyard :: General, enhancement)

Other
Maemo
enhancement
Not set

Tracking

(fennec1.0-)

VERIFIED FIXED
fennec1.0b1
Tracking Status
fennec 1.0- ---

People

(Reporter: christian.bugzilla, Assigned: madhava)

References

Details

(Keywords: uiwanted)

Attachments

(4 files, 3 obsolete files)

No description provided.
Keywords: uiwanted
Target Milestone: --- → Fennec A2
If a site opens popups (which are displayed as tabs) the user needs to be informed
Summary: Inform the user when a new browser window is opened → Inform the user when a new tab is opened
Flags: wanted-fennec1.0?
Target Milestone: Fennec A2 → Fennec A3
we do support the "TabOpen" event so add-ons or other code could watch for it and display some UI.
You don't think a notification would be useful?
(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.
(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.
I think (comment #3) sounds like a good idea, was just worried that comment #2 implied to leave it up to add-ons.
Blocks: 477628
Flags: wanted-fennec1.0? → wanted-fennec1.0+
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.
tracking-fennec: --- → ?
Assignee: nobody → madhava
tracking-fennec: ? → 1.0-
Attached patch Patch v0.1 (obsolete) — Splinter Review
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.
Attachment #395735 - Flags: review?
Attached patch Add the css rule for wince theme (obsolete) — Splinter Review
Attachment #395735 - Attachment is obsolete: true
Attachment #395805 - Flags: review?(mark.finkle)
Attachment #395735 - Flags: review?
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
Attachment #395805 - Flags: review?(mark.finkle) → review-
Attached patch Patch v0.2 (obsolete) — Splinter Review
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.
Attachment #395805 - Attachment is obsolete: true
Attached patch Patch v0.3Splinter Review
The patch is using PluralForm.jsm now
Attachment #396408 - Attachment is obsolete: true
pushed: https://hg.mozilla.org/mobile-browser/rev/4dd9059d0448
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Verified with 20091001 1.9.2 beta4 candidate on my n810
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.