Last Comment Bug 474056 - Implement optional taskbar preview-per-tab
: Implement optional taskbar preview-per-tab
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Shell Integration (show other bugs)
: Trunk
: All Windows 7
: P1 normal with 6 votes (vote)
: Firefox 3.7a1
Assigned To: Rob Arnold [:robarnold]
:
Mentors:
: 474054 544271 (view as bug list)
Depends on: 522035 522855 524468 527181 538487 591135 592451 616720 501490 503355 508850 508902 520579 520724 520801 520837 520937 520943 521188 521216 521668 521954 522262 522305 522416 522506 522610 522659 524316 524633 524952 524982 525810 526618 526620 526693 526984 527103 527105 527156 527158 527268 527914 527974 528761 529211 544924 545077 550763 551597 551925 559613 562253 563337 580522
Blocks: win7support 557924 474054 520807 525823
  Show dependency treegraph
 
Reported: 2009-01-16 15:25 PST by Ed Lee :Mardak
Modified: 2015-01-16 00:12 PST (History)
38 users (show)
mbeltzner: blocking‑firefox3.6+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed


Attachments
v1.0 (23.62 KB, patch)
2009-07-22 15:14 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Splinter Review
v1.5 (27.55 KB, patch)
2009-08-06 14:37 PDT, Rob Arnold [:robarnold]
dao+bmo: review-
Details | Diff | Splinter Review
v2.0 (40.46 KB, patch)
2009-10-04 22:49 PDT, Rob Arnold [:robarnold]
rflint: review+
dao+bmo: review+
Details | Diff | Splinter Review
v2.1 (42.39 KB, patch)
2009-10-05 19:19 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Splinter Review
test fix for non-windows 7 machines (796 bytes, patch)
2009-10-06 00:20 PDT, Rob Arnold [:robarnold]
no flags Details | Diff | Splinter Review
Small bug fixes that should have been caught before checkin (2.00 KB, patch)
2009-10-06 11:04 PDT, Rob Arnold [:robarnold]
dao+bmo: review+
Details | Diff | Splinter Review

Description Ed Lee :Mardak 2009-01-16 15:25:37 PST
Related to bug 474054 where we show a preview of the tabs, but this is to actually show the full size contents of the actual tab (incase another tab is actually focused in that window).

This is supposed to be just a preview, so we probably don't want to actually switch tabs for the user when the hover leaves the tab preview without selecting a tab.
Comment 1 Rob Arnold [:robarnold] 2009-01-16 18:55:23 PST
It looks like ITaskbar3::RegisterTab wants a native window for every tab. I don't think we currently do this.
Comment 2 Ed Lee :Mardak 2009-01-16 20:10:50 PST
So we can't just show the window (native window?) containing the tab and internally switch the current tab to be the desired tab?
Comment 3 sibbl 2009-01-29 07:11:21 PST
In my opinion it should be possible to deactivate this. I hate this feature in IE because I don't want to click on the taskbar item first, look for my last used tab and click again on it. I just want to click on the icon and have the Firefox window in front of me!
Maybe it would be possible to list the opened tabs in the jump list?
Comment 4 Matthew Brown 2009-03-13 17:56:01 PDT
(In reply to comment #3)
> In my opinion it should be possible to deactivate this. I hate this feature in
> IE because I don't want to click on the taskbar item first, look for my last
> used tab and click again on it. I just want to click on the icon and have the
> Firefox window in front of me!
From my limited usage of IE8 on Windows 7, I agree. Tab switching should be left to the application or at least be optional. However, I believe your comment would be more suitable for bug 474054.
Comment 5 redsign 2009-03-15 13:37:46 PDT
That's why it can be disabled via the internet options for IE and I would do that if I use it. But I think it would be good to have that option for those who like it.
Comment 6 Rob Arnold [:robarnold] 2009-07-09 11:22:12 PDT
*** Bug 474054 has been marked as a duplicate of this bug. ***
Comment 7 Rob Arnold [:robarnold] 2009-07-09 11:29:24 PDT
Changing the name to better reflect the intention to create a front end feature (bug 501490 adds the backend support).
Comment 8 Rob Arnold [:robarnold] 2009-07-22 15:14:44 PDT
Created attachment 390086 [details] [diff] [review]
v1.0

First RC. There is still a bug when activating a minimized window's tabs but I'm not sure where the bug is for that (could be in the backend).
Comment 9 Jim Mathies [:jimm] 2009-07-23 07:46:42 PDT
Rob, maybe we should change the aeropeek component to something generic like taskbar? I'm no longer in need of something in mozapps, but now I'll need a place up in browser for jump lists.
Comment 10 Rob Arnold [:robarnold] 2009-07-23 14:00:51 PDT
How does wintaskbar sound? It's consistent with the interface name.
Comment 11 Jim Mathies [:jimm] 2009-07-23 14:10:20 PDT
(In reply to comment #10)
> How does wintaskbar sound? It's consistent with the interface name.

WFM!
Comment 12 :Gavin Sharp [email: gavin@gavinsharp.com] 2009-07-27 16:31:16 PDT
Dao may be interested in reviewing this.
Comment 13 Dão Gottwald [:dao] 2009-07-29 05:14:47 PDT
Why are you selecting the tab to preview it? I don't think we should do that, given the tabbrowser.xml hacks it implies.
Comment 14 Dão Gottwald [:dao] 2009-07-29 05:44:14 PDT
Is the expected UI documented somewhere, btw? I'm having problems getting any previews from IE8 on Win7 RC1.
Comment 15 Sylvain Pasche 2009-07-29 06:35:01 PDT
With IE it should look like http://res1.windows.microsoft.com/resbox/en/Windows%207/Main/3/8/385c8434-d335-4ce5-8925-7d2357ba9ca4/385c8434-d335-4ce5-8925-7d2357ba9ca4.jpg

You may not see the previews if you don't have Aero turned on (which can happen if you're running Win7 in a vm where there is no hardware acceleration).
Comment 16 Dão Gottwald [:dao] 2009-07-29 06:35:59 PDT
Right, this is a VM :(
Comment 17 :Gavin Sharp [email: gavin@gavinsharp.com] 2009-07-29 09:19:50 PDT
(In reply to comment #13)
> Why are you selecting the tab to preview it? I don't think we should do that,
> given the tabbrowser.xml hacks it implies.

Rob and I discussed this. I think it's the only way to achieve the affect that we want. In particular, we wanted most of the browser UI to be displayed as though the tab was selected (back/forward state, selected tab in the tab bar, URL bar value, etc.). Alternate suggestions would be welcome, though.
Comment 18 Dão Gottwald [:dao] 2009-07-29 12:31:26 PDT
Ok, now that I've watched a video on aero peek, I understand the desired effect.

I guess we could live with maintaining that mode in tabbrowser. The TabSelect shouldn't be dispatched, though. Otherwise you would have to update the code that's listening to that event, and extensions would have to do that as well.
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2009-07-29 13:09:24 PDT
Yeah, that makes sense.
Comment 20 Rob Arnold [:robarnold] 2009-07-29 13:18:05 PDT
I didn't know if extensions might want to update their UI depending on the selected tab. I think we'd want to preview that.
Comment 21 Jim Jeffery not reading bug-mail 1/2/11 2009-07-29 14:10:54 PDT
Perhaps I'm mis-reading, but what good is the Aero-tab-Preview, if clicking on the tab doesn't select the highlighted tab ?  Some may not want to inconvience of the extra click, or clicking on the task-bar, but there are those that will use the feature.  Just displaying a panel of tabs however, and not being able to select that tab is a waste of coding efforts IMO.  If the intent is to only display the tab, then there needs to be a way to 'kill' the feature as its useless.
Comment 22 Dão Gottwald [:dao] 2009-07-29 14:18:00 PDT
(In reply to comment #20)
> I didn't know if extensions might want to update their UI depending on the
> selected tab. I think we'd want to preview that.

I think we can live with not previewing that, for the sake of not breaking other stuff. There are definitely TabSelect-dependent actions that should not happen in this case, comparable to the stuff that you made mInPreview-dependent.
Comment 23 Rob Arnold [:robarnold] 2009-07-29 14:21:17 PDT
(In reply to comment #21)
> Perhaps I'm mis-reading, but what good is the Aero-tab-Preview, if clicking on
> the tab doesn't select the highlighted tab ?

Clicking on the tab will select the tab. Previewing a tab will not leave that tab selected however. This feature operates the same way that it does for IE8 with exception that we update the browser chrome when previewing a tab.

> If the intent is to only display the tab, then there needs to be a way to
> 'kill' the feature as its useless.

There is a hidden pref, aeropeek.enable, to turn the feature off. No restart required either.

(In reply to comment #22)
> I think we can live with not previewing that, for the sake of not breaking
> other stuff. There are definitely TabSelect-dependent actions that should not
> happen in this case, comparable to the stuff that you made
> mInPreview-dependent.

Ok, sounds fine then.
Comment 24 [not reading bugmail] 2009-07-31 19:30:41 PDT
I find it very interesting bug 465076 to implement Ctrl+Tab view is almost the same functionality built directly into the browser.
Comment 25 John Mellor (Jomel) 2009-08-02 10:19:56 PDT
I'd recommend mentioning the fake tab selections in the dev docs (even if you don't send TabSelect events, some extensions may get confused by this).
Comment 26 Rob Arnold [:robarnold] 2009-08-06 14:37:36 PDT
Created attachment 393028 [details] [diff] [review]
v1.5

Now with more comments and (hopefully) better style.
Moved from browser/components/aeropeek to browser/components/wintaskbar.
Simplified some code (thumbnails are now drawn from the previews) and other minor polish work.
Comment 27 Mike Beltzner [:beltzner, not reading bugmail] 2009-08-31 08:34:41 PDT
Needed for Windows 7 parity with Chrome, IE and Safari
Comment 28 Rob Arnold [:robarnold] 2009-09-24 16:03:45 PDT
Comment on attachment 393028 [details] [diff] [review]
v1.5

No tests yet, but with the upcoming code freeze I'd like some review so that this might make it in for beta.
Comment 29 Dão Gottwald [:dao] 2009-09-25 05:41:22 PDT
Comment on attachment 393028 [details] [diff] [review]
v1.5

Note that I cannot test this, since I have Win 7 only on a VM. So I don't feel comfortable being the only reviewer.

You're putting modules in browser/components/. You should probably use browser/modules/ instead.

>+      <field name="mInPreview">
>+        false
>+      </field>
>+
>+      <property name="previewmode">
>+        <getter>
>+          return this.mInPreview;
>+        </getter>
>+        <setter>
>+          <![CDATA[
>+          this.mInPreview = val;
>+          return val;
>+          ]]>
>+        </setter>
>+      </property>

Just make this one public field, previewMode. No need for a private field and a public property.

>+//// WinTaskbarHelper
>+
>+/*
>+ * This object is for helping clients of the nsIWinTaskbar initialize
>+ * themselves. The taskbar service is not initially available - it requires
>+ * Windows to notify us when it's ready to accept API calls.

Are we going to have multiple nsIWinTaskbar clients?

>+var WinTaskbarHelper = {

>+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver]),

I don't think you need to implement QueryInterface here.

>+++ b/browser/components/wintaskbar/preview-per-tab.jsm

>+// Pref to enable/disable preview-per-tab
>+const kTogglePrefName = "aeropeek.enable";

constants should be uppercase without the k prefix, e.g. TOGGLE_PREF_NAME

>+//// Various utility functions
>+var Util = {
>+  get ioSvc () {
>+    delete this.ioSvc;
>+    return this.ioSvc = Cc["@mozilla.org/network/io-service;1"]
>+                       .getService(Ci.nsIIOService);
>+  },
[...]

I'm not sure that the Util wrapper is useful, you could just dump this stuff in the global scope.

E.g.

XPCOMUtils.defineLazyServiceGetter(this, "_ioService",
                                   "@mozilla.org/network/io-service;1",
                                   "nsIIOService");
...
function _imageFromURI(uri) {
  ...
}

>+var PreviewController = function (win, tab) {
>+  this.win = win.QueryInterface(Ci.nsIDOMWindowInternal);;
>+  this.tab = tab;
>+  let linkedBrowser = tab.linkedBrowser;
>+  this.tabbrowser = linkedBrowser.getTabBrowser();
>+  this.shell = win.gBrowser.docShell;

Use win.gBrowser instead of linkedBrowser.getTabBrowser().

>+  this.canvasPreview =
>+    doc.createElementNS("http://www.w3.org/1999/xhtml", "html:canvas");

Just "canvas", without "html:".

>+  get wrappedJSObject() {
>+    return this;
>+  },

I'm not sure I understand your use of wrappedJSObject. Can you just get rid of it?

>+  onTabPaint: function (rect) {
>+    // Ignore spurious dirty rects
>+    if (!rect.width || !rect.height) return;

\n before return

>+  onLocationChange: function (aBrowser, webProgress, request, location) {
>+  },
>+  onProgressChange: function (aBrowser, webProgress, request, curSelfProgress,
>+                              maxSelfProgress, curTotalProgress,
>+                              maxTotalProgress) {
>+  },
>+  onSecurityChange: function (aBrowser, aWebProgress, aRequest, aState) {
>+  },
>+  onStateChange: function (aBrowser, aWebProgress, aRequest,
>+                           aStateFlags, aStatus) {
>+  },
>+  onStatusChange: function (aBrowser, aWebProgress, aRequest,
>+                            aStatus, aMessage) {
>+  },

You can spare the argument lists.

>+  onRefreshAttempted: function (aBrowser, webProgress, aRefreshURI,
>+                                aMillis, aSameURI) {
>+    return true;
>+  },

You don't need to implement onRefreshAttempted.

>+TabWindow.prototype = {

>+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIDOMEventListener]),

Also not needed.

>+  // nsIDOMEventListener
>+  handleEvent: function (evt) {
>+    switch(evt.type) {
>+      case "TabOpen":
>+      case "TabClose":
>+      case "TabSelect":
>+      case "TabMove":
>+        let tab, preview;
>+        tab = evt.originalTarget;
>+        preview = AeroPeek.previews[tab];

Move this up and reduce the two switches to one?

>+          case "TabMove":
>+            let controller = preview.controller.wrappedJSObject;
>+            let idx = Array.indexOf(controller.tabbrowser.mTabs, tab);
>+            let next = null
>+            // Check if there is a next preview
>+            if (idx+1 < controller.tabbrowser.mTabs.length) {
>+              next = AeroPeek.previews[controller.tabbrowser.mTabs[idx+1]];
>+            }

use tab.nextSibling instead of controller.tabbrowser.mTabs[idx+1]

>+var EXPORTED_SYMBOLS = ["AeroPeek"]

I think this belongs to the top of the file. Also add a semicolon.
Comment 30 Dão Gottwald [:dao] 2009-09-25 06:57:32 PDT
Comment on attachment 393028 [details] [diff] [review]
v1.5

>+function TabWindow(win) {

>+  for each (let evtName in this.events) {
>+    tabbrowser.addEventListener(evtName, this, false);

add the event listeners on tabbrowser.tabContainer

Also please drop the curly brackets around single lines...
Comment 31 Rob Arnold [:robarnold] 2009-10-04 22:49:49 PDT
Created attachment 404560 [details] [diff] [review]
v2.0

Notable changes since last patch:
Review comments addressed (dao's and others from IRC/email).
init.jsm removed due to underlying API changes in bug 501490's latest patch.
Large block of module documentation in preview-per-tab.jsm.

Feature changes:
Time-based tab content cache expiration.
Magic number based auto-disabling (see large comment in preview-per-tab.jsm).

Implementation changes:
Removed global mapping of <tab> to nsITaskbarPreview. There is now a mapping in each TabWindow of <tab> to nsITaskbarPreview via the tab position.
The ChromeWindow to TabWindow mapping is now done via property of the ChromeWindow (used only in the case of a closing window).
Lots more lazy getters to avoid new window/tab performance regressions.
Dirty regions are now clipped to the canvas area to avoid some spurious rectangles causing drawWindow to throw.
Tab ordering refreshed for all tabs in a window after move/open/close operations.
General code cleanup and simplification.
Tests now included.
Probably a few other things I've forgotten to mention here.
Comment 32 Rob Arnold [:robarnold] 2009-10-05 07:15:13 PDT
Comment on attachment 404560 [details] [diff] [review]
v2.0

Switching review to Rob Strong per conversation with Dão on IRC.
Comment 33 Jim Mathies [:jimm] 2009-10-05 08:06:01 PDT
Rob, we need to sync up on where this code lands. I've currently got:

components/taskbar

with make clipping out the taskbar dir in browser/components/Makefile.in.

The install location I have is dist/bin/modules/(jsm files).

Any issues with that?
Comment 34 Dão Gottwald [:dao] 2009-10-05 08:06:05 PDT
Comment on attachment 404560 [details] [diff] [review]
v2.0

>+      <field name="previewmode">

nit: previewMode

>+DIRS += wintaskbar

Jim uses "taskbar"... Please make sure that you're on the same page.

>+// Pref to enable/disable preview-per-tab
>+const TOGGLE_PREF_NAME = "aeropeek.enable";
>+// Pref to determine the magic auto-disable threshold
>+const DISABLE_THRESHOLD_PREF_NAME = "aeropeek.maxpreviews";
>+// Pref to control the time in seconds that tab contents live in the cache
>+const CACHE_EXPIRATION_TIME_PREF_NAME = "aeropeek.cachetime";

Did you intentionally not add default values for these prefs? It seems like you should (and then don't try/catch when you read them).

+function PreviewController(win, tab) {

+  win.tabbrowser.addTabsProgressListener(this);

Can you do this only once per window?

>+  updateCanvasPreview: function () {
>+    let win = this.linkedBrowser.contentWindow.QueryInterface(Ci.nsIDOMWindowInternal);

You don't seem to be using nsIDOMWindowInternal.

>+      case "DOMTitleChanged":
>+        // The tab's label is sometimes empty when dragging tabs between windows
>+        let htmldoc = evt.originalTarget.defaultView.top.document;
>+        let title = htmldoc.title || '(Untitled)';

You shouldn't hardcode '(Untitled)'. Perhaps calling tabbrowser.setTabTitle(tab) fixes the issue? And add a comment with a bug number for the empty label bug.

>+  onLinkIconAvailable: function (aBrowser) {
>+    if (aBrowser != this.tab.linkedBrowser)
>+      return;
>+    let img = getFaviconAsImage(aBrowser.mIconURL);
>+    this.preview.icon = img

add a semicolon

>+XPCOMUtils.defineLazyGetter(PreviewController.prototype, "canvasPreviewFlags",
>+  function () { let canvasInterface = Ci.nsIDOMCanvasRenderingContext2D;
>+                return canvasInterface.DRAWWINDOW_DRAW_VIEW
>+                     | canvasInterface.DRAWWINDOW_DRAW_CARET
>+                     | canvasInterface.DRAWWINDOW_DO_NOT_FLUSH;
>+});

This is only used once, and you have a context there, so you could probably just do this:

let flags = ctx.DRAWWINDOW_DRAW_VIEW
          | ctx.DRAWWINDOW_DRAW_CARET
          | ctx.DRAWWINDOW_DO_NOT_FLUSH;

instead of:

let flags = this.canvasPreviewFlags;

>+  handleEvent: function (evt) {
>+    let tab = evt.originalTarget;
>+    switch(evt.type) {

nit: add a space after 'switch'

>+function TabWindow(win) {
>+  this.win = win.QueryInterface(Ci.nsIDOMWindowInternal);

Another useless QueryInterface?

Otherwise looks good to me code-wise, but I can't test this, so you should definitely try to get a second review.
Comment 35 Ryan Flint [:rflint] (ping via IRC for reviews) 2009-10-05 16:25:26 PDT
Comment on attachment 404560 [details] [diff] [review]
v2.0

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js

>+#ifdef AERO_PEEK
>+  Cu.import("resource://gre/modules/wintaskbar/preview-per-tab.jsm");
>+
>+  AeroPeek.onOpenWindow(window);
>+#endif
> }
> 
> function BrowserShutdown()
> {
>+#ifdef AERO_PEEK
>+  AeroPeek.onCloseWindow(window);
>+#endif

I'd like to see on(Open|Close)Window wrapped in something that can handle the .available 
logic and not bother importing the module if we're on an unsupported windows version.

>diff --git a/browser/components/wintaskbar/preview-per-tab.jsm b/browser/components/wintaskbar/preview-per-tab.jsm

>+ * This module implements the front end behavior for AeroPeek. Starting in
>+ * Windows Vista, the taskbar begin showing live thumbanil previews of windows

nit: began/thumbnail

>+var EXPORTED_SYMBOLS = ["AeroPeek"]

nit: missing semicolon

>+Cu.import("resource://gre/modules/NetUtil.jsm");

>+//// Various utility properties
>+XPCOMUtils.defineLazyServiceGetter(this, "ioSvc",
>+                                   "@mozilla.org/network/io-service;1",
>+                                   "nsIIOService");

Purely followup fodder: NetUtil now has an ioService property, but it's only on trunk.

>+function _imageFromURI(uri) {
>+  let channel = ioSvc.newChannelFromURI(uri);
>+
>+  let out_img = { value: null };
>+  let inputStream = channel.open();
>+  try {
>+    imgTools.decodeImageData(inputStream, channel.contentType, out_img);

I noticed a couple sites (BBC, f.e.) that trip this up by serving their favicon with a non-image-y MIME type.
Maybe we want to attempt (in a followup) overriding the server's type with image/x-icon or falling back to asking the favicon service.

>+TabWindow.prototype = {
>+  _enabled: false,
>+  events: ["TabOpen", "TabClose", "TabSelect", "TabMove"],

Probably want to listen for and invalidate the taskbar previews on window resizes as well.
When I was testing this, only the active tab preview was invalidated, leaving differently sized thumbnails that changed size on hover.

Looks good!
Comment 36 Rob Arnold [:robarnold] 2009-10-05 19:19:13 PDT
Created attachment 404750 [details] [diff] [review]
v2.1

Updated per Dão and Ryan's comments.
Comment 37 Rob Arnold [:robarnold] 2009-10-05 22:02:59 PDT
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/a949266351ca
Pushed to mozilla-1.9.2:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7fe92f6dc3dc
Comment 38 Rob Arnold [:robarnold] 2009-10-06 00:20:10 PDT
Created attachment 404786 [details] [diff] [review]
test fix for non-windows 7 machines

This fixes the mochitests when run on machines that aren't Windows 7. Wasn't caught by earlier run through try server because previous versions of the patch always had AeroPeek defined.
Comment 39 Marco Bonardo [::mak] (Away 6-20 Aug) 2009-10-06 05:49:52 PDT
i've pushed this additional changeset on central to fix persistent timeout
http://hg.mozilla.org/mozilla-central/rev/d6214cab6ad5
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d889630eac9d
Comment 40 Jim Mathies [:jimm] 2009-10-06 08:59:03 PDT
While I was playing around with the jump lists patch, I noticed this error show up in the error console:

Error: win is not defined
Source File: file:///C:/Mozilla/Firefox/473045/mozilla/BUILD-DEBUG/dist/bin/modules/wintaskbar/preview-per-tab.jsm
Line: 399
Comment 41 Jim Mathies [:jimm] 2009-10-06 09:02:32 PDT
(In reply to comment #40)
> While I was playing around with the jump lists patch, I noticed this error show
> up in the error console:
> 
> Error: win is not defined
> Source File:
> file:///C:/Mozilla/Firefox/473045/mozilla/BUILD-DEBUG/dist/bin/modules/wintaskbar/preview-per-tab.jsm
> Line: 399

and this one:

Error: preview is undefined
Source File: file:///C:/Mozilla/Firefox/473045/mozilla/BUILD-DEBUG/dist/bin/modules/wintaskbar/preview-per-tab.jsm
Line: 432

This error seems to happen if you have other windows besides the main browser window open and you close the browser window.

I haven't been able to reproduce the first yet.
Comment 42 Gary [:streetwolf] 2009-10-06 10:30:03 PDT
running W7 RTM and I do not get taskbar thumbnail previews for individual tabs.  And btw, Aeropeek is the ability in W7 to hover over the box at the very right of the taskbar to make all windows transparent in order to see the desktop.  It has nothing to do with taskbar thumbnail previews.
Comment 43 Rob Arnold [:robarnold] 2009-10-06 11:04:39 PDT
Created attachment 404861 [details] [diff] [review]
Small bug fixes that should have been caught before checkin

Both errors fixed - this should have been caught before checkin.
Comment 44 Rob Arnold [:robarnold] 2009-10-06 11:09:11 PDT
(In reply to comment #42)
> running W7 RTM and I do not get taskbar thumbnail previews for individual tabs.

How many tabs do you have? If you have more than 20, the preview-per-tab feature is disabled to avoid cluttering your UI. Change aeropeek.maxpreviews to modify the threshold.

>  And btw, Aeropeek is the ability in W7 to hover over the box at the very right
> of the taskbar to make all windows transparent in order to see the desktop.  It
> has nothing to do with taskbar thumbnail previews.

The Engineering Windows 7 blog does indeed refer to the live preview as Aero Peek (see http://blogs.msdn.com/e7/archive/2009/02/26/some-changes-since-beta.aspx). The button the taskbar is also part of the Aero Peek feature.
Comment 45 Gary [:streetwolf] 2009-10-06 11:32:03 PDT
It doesn't make a difference how many tabs I have.  I only see the current tab.

Let's make sure I got this fix right.  If I have two tabs in FF and I hover over the Windows taskbar I should see thumbnails of both tabs, just like I do when I have multiple FF windows opened, correct?

The term Aero peek has different meanings depending on where you look.  The option for turning AP on is described as "Use Aero Peek to preview the desktop".

I like to call it thumbnail previews just as it's always been.  I suppose the added ability to hover over the thumbnail and see it fullscreen is sort of a "Peek".

Anyways, I can't get it working if I understand it the way I stated.
Comment 46 Rob Arnold [:robarnold] 2009-10-06 11:43:20 PDT
(In reply to comment #45)
> It doesn't make a difference how many tabs I have.  I only see the current tab.
> 
> Let's make sure I got this fix right.  If I have two tabs in FF and I hover
> over the Windows taskbar I should see thumbnails of both tabs, just like I do
> when I have multiple FF windows opened, correct?

It works for me on the nightly trunk build from this morning. What build are you running? Do you see any errors in the Error Console?
Comment 47 Gary [:streetwolf] 2009-10-06 11:49:53 PDT
i'm on branch which states the fix is there.

 Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2b1pre) Gecko/20091006 Namoroka/3.6b1pre Firefox/3.5.3 - Build ID: 20091006045110

nothing in the error console
Comment 48 Dão Gottwald [:dao] 2009-10-06 11:59:41 PDT
Comment on attachment 404861 [details] [diff] [review]
Small bug fixes that should have been caught before checkin

>     for (let i = 0; i < tabs.length; i++)
>-      this.removeTab(tabs[i]);
>+      this.removeTab(tabs[i], true);

Set this._destroying instead of adding the dontSplice argument.
Comment 49 Gary [:streetwolf] 2009-10-06 12:10:03 PDT
OK, I must be confusing this fix with https://bugzilla.mozilla.org/show_bug.cgi?id=501490 which did come in with last nights trunk.
Comment 50 Gary [:streetwolf] 2009-10-06 12:37:18 PDT
Tried Trunk and tab previews works!  Only thing I found is that when first hovering over a preview thumbnail the screen flickers.
Comment 51 Darren Kalck [:D-Kalck] 2009-10-06 12:58:50 PDT
(In reply to comment #50)
> Tried Trunk and tab previews works!  Only thing I found is that when first
> hovering over a preview thumbnail the screen flickers.

I have the same problem, so I tried with IE8.
With IE8 when you hover a tab preview, only the content (and the window caption) changes.
Comment 52 DG 2009-10-09 07:07:54 PDT
The message "tab is undefined" keeps showing up in the error console and the previews just show the standard win7 loading icon.


Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091008 Minefield/3.7a1pre
Comment 53 Jim Jeffery not reading bug-mail 1/2/11 2009-10-09 07:11:04 PDT
(In reply to comment #52)
> The message "tab is undefined" keeps showing up in the error console and the
> previews just show the standard win7 loading icon.
> 
> 
> Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091008
> Minefield/3.7a1pre

See bug: https://bugzilla.mozilla.org/show_bug.cgi?id=521216 which has a fix pending approval.  Not sure if this fix will fix the issue of the loading icon showing up or not, may be another bug.
Comment 54 Rimas Kudelis 2009-10-14 08:09:25 PDT
This only partly works in 3.6pre. Out of the ten tabs that I have, only the last one currently has a preview, all others just display a progress circle. New tabs get a preview though. Could it be that the previews are never generated for startup tabs or something?
Comment 55 Rob Arnold [:robarnold] 2009-10-14 08:33:52 PDT
Previews are generated on demand. Is there anything in your error console? Also, if you wait a minute or two (without having clicked the taskbar icon), do the progress circles resolve into a preview?

Please followup to bug 522262.
Comment 56 Rimas Kudelis 2009-10-19 23:14:03 PDT
Just wondering: both Aero Peek and Jumplists seem to have been disabled in latest mozilla-1.9.2 nightlies. Is that the case, or has something just broke?
Comment 57 [not reading bugmail] 2009-10-22 12:49:13 PDT
(In reply to comment #56)
> Just wondering: both Aero Peek and Jumplists seem to have been disabled in
> latest mozilla-1.9.2 nightlies. Is that the case, or has something just broke?

Jumplists are not yet landed on 1.9.2; see bug 473045, bug 518666 comment 81 and bug 521304 comment 9.

Aero peek for tab previews are dependent on about:config preferences browser.taskbar.*  and the width of your display and the number of tabs you open.

For example if the using the default, you might find 8 previews at 1024x768, adding more moves it the list view upto the max, then disappears all together after that, no list, no previews.  Loading a ton of bookmarks via "open all in tabs" > max list only seems to preview the first tab or runs the window preview again vs tab previews.

Also stuck aero peek tab previews is still open in bug 522506 on both trunk and branch.
Comment 58 [not reading bugmail] 2009-10-22 13:02:41 PDT
(In reply to comment #57)
> (In reply to comment #56)
> > Is that the case, or has something just broke?
> 
> Also stuck aero peek tab previews is still open in bug 522506 on both trunk and
> branch.

Oops.. that should be bug 522262, and bug 522506 is actually for aero peek previews resolving into generic thumbnail icons.

What was fixed recently was bug 522416, which resolved loading http requests as async vs sync so we don't see all aero peek previews running together which caused the browser bugs like comment 54 and caused the browser to eventually hang.
Comment 59 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-02-04 09:30:13 PST
*** Bug 544271 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.