Implement optional taskbar preview-per-tab

RESOLVED FIXED in Firefox 3.7a1

Status

()

Firefox
Shell Integration
P1
normal
RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: Mardak, Assigned: robarnold)

Tracking

(Depends on: 8 bugs, Blocks: 2 bugs)

Trunk
Firefox 3.7a1
All
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3.6 +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

9 years ago
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.
(Assignee)

Comment 1

9 years ago
It looks like ITaskbar3::RegisterTab wants a native window for every tab. I don't think we currently do this.
(Reporter)

Comment 2

9 years ago
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

9 years ago
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

8 years ago
(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

8 years ago
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.
(Assignee)

Updated

8 years ago
Assignee: nobody → tellrob
Status: NEW → ASSIGNED
(Assignee)

Updated

8 years ago
Depends on: 501490
(Assignee)

Updated

8 years ago
Duplicate of this bug: 474054
(Assignee)

Comment 7

8 years ago
Changing the name to better reflect the intention to create a front end feature (bug 501490 adds the backend support).
Hardware: x86 → All
Summary: Show tab when hovering over tab preview of taskbar preview → Implement optional taskbar preview-per-tab
(Assignee)

Updated

8 years ago
Depends on: 503355
(Assignee)

Comment 8

8 years ago
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).
Attachment #390086 - Flags: review?
(Assignee)

Updated

8 years ago
Attachment #390086 - Flags: review? → review?(gavin.sharp)

Comment 9

8 years ago
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.
(Assignee)

Comment 10

8 years ago
How does wintaskbar sound? It's consistent with the interface name.

Comment 11

8 years ago
(In reply to comment #10)
> How does wintaskbar sound? It's consistent with the interface name.

WFM!
(Assignee)

Updated

8 years ago
Attachment #390086 - Flags: review?(mconnor)
Dao may be interested in reviewing this.
Why are you selecting the tab to preview it? I don't think we should do that, given the tabbrowser.xml hacks it implies.
Is the expected UI documented somewhere, btw? I'm having problems getting any previews from IE8 on Win7 RC1.

Comment 15

8 years ago
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).
Right, this is a VM :(
(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.
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.
Yeah, that makes sense.
(Assignee)

Comment 20

8 years ago
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.
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.
(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.
(Assignee)

Comment 23

8 years ago
(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.
I find it very interesting bug 465076 to implement Ctrl+Tab view is almost the same functionality built directly into the browser.

Comment 25

8 years ago
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).
(Assignee)

Comment 26

8 years ago
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.
Attachment #390086 - Attachment is obsolete: true
Attachment #393028 - Flags: review?(gavin.sharp)
Attachment #390086 - Flags: review?(mconnor)
Attachment #390086 - Flags: review?(gavin.sharp)
(Assignee)

Updated

8 years ago
Attachment #393028 - Flags: review?(gavin.sharp) → review?(dao)
(Assignee)

Updated

8 years ago
Attachment #393028 - Flags: review?(dao)
Needed for Windows 7 parity with Chrome, IE and Safari
Flags: blocking-firefox3.6+
Priority: -- → P1
(Assignee)

Comment 28

8 years ago
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.
Attachment #393028 - Flags: review?(dao)
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.

Updated

8 years ago
Attachment #393028 - Flags: review?(dao) → review-
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...
(Assignee)

Comment 31

8 years ago
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.
Attachment #393028 - Attachment is obsolete: true
Attachment #404560 - Flags: review?(rflint)
(Assignee)

Updated

8 years ago
Attachment #404560 - Flags: review?(dao)
(Assignee)

Comment 32

8 years ago
Comment on attachment 404560 [details] [diff] [review]
v2.0

Switching review to Rob Strong per conversation with Dão on IRC.
Attachment #404560 - Flags: review?(dao) → review?(robert.bugzilla)

Comment 33

8 years ago
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 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.
Attachment #404560 - Flags: review+
Attachment #404560 - Flags: review?(rflint) → review+
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!
Whiteboard: [has patch][needs review rs]
(Assignee)

Comment 36

8 years ago
Created attachment 404750 [details] [diff] [review]
v2.1

Updated per Dão and Ryan's comments.
Attachment #404560 - Attachment is obsolete: true
Attachment #404560 - Flags: review?(robert.bugzilla)
(Assignee)

Updated

8 years ago
Whiteboard: [has patch][needs review rs] → [has patch][needs tabbrowser.xml review]
(Assignee)

Comment 37

8 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
status1.9.2: --- → beta1-fixed
Resolution: --- → FIXED
Whiteboard: [has patch][needs tabbrowser.xml review]
Target Milestone: --- → Firefox 3.6b1
(Assignee)

Updated

8 years ago
Depends on: 508850

Updated

8 years ago
Depends on: 520724
(Assignee)

Comment 38

8 years ago
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.
Attachment #404786 - Flags: review?(jmathies)
(Assignee)

Updated

8 years ago
Attachment #404786 - Flags: review?(jmathies)
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

8 years ago
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

8 years ago
(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.

Updated

8 years ago
Depends on: 520801

Comment 42

8 years ago
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.
(Assignee)

Comment 43

8 years ago
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.
Attachment #404861 - Flags: review?(dao)
(Assignee)

Comment 44

8 years ago
(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

8 years ago
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.
(Assignee)

Comment 46

8 years ago
(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

8 years ago
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
(Assignee)

Updated

8 years ago
Attachment #404861 - Flags: review?(dao) → review?(rflint)

Updated

8 years ago
Attachment #404861 - Flags: review?(rflint) → review+
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.
(Assignee)

Updated

8 years ago
Depends on: 520837

Comment 49

8 years ago
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.

Updated

8 years ago
Blocks: 520807

Comment 50

8 years ago
Tried Trunk and tab previews works!  Only thing I found is that when first hovering over a preview thumbnail the screen flickers.
(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.
Target Milestone: Firefox 3.6b1 → Firefox 3.7a1
Depends on: 520937

Updated

8 years ago
Depends on: 520943

Updated

8 years ago
Depends on: 521188

Updated

8 years ago
Depends on: 521216

Comment 52

8 years ago
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
(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.
(Assignee)

Updated

8 years ago
Depends on: 508902
Depends on: 522035

Updated

8 years ago
Depends on: 521954

Comment 54

8 years ago
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?
(Assignee)

Comment 55

8 years ago
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.
Depends on: 522262

Updated

8 years ago
Depends on: 522305
Depends on: 522416

Updated

8 years ago
Component: Tabbed Browser → Shell Integration
QA Contact: tabbed.browser → shell.integration

Updated

8 years ago
Depends on: 522506

Updated

8 years ago
Depends on: 522610

Updated

8 years ago
Depends on: 522855

Comment 56

8 years ago
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?
(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.
(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.

Updated

8 years ago
Depends on: 524316

Updated

8 years ago
Depends on: 524468
Depends on: 524633
Depends on: 524982

Updated

8 years ago
Depends on: 525810
Depends on: 522659

Updated

8 years ago
Depends on: 526618

Updated

8 years ago
Depends on: 526620

Updated

8 years ago
Depends on: 526693
Depends on: 521668

Updated

8 years ago
Depends on: 527103

Updated

8 years ago
Blocks: 527105

Updated

8 years ago
No longer blocks: 527105
Depends on: 527105

Updated

8 years ago
Depends on: 524952
(Assignee)

Updated

8 years ago
Depends on: 526984

Updated

8 years ago
Depends on: 527156

Updated

8 years ago
Depends on: 527158
(Assignee)

Updated

8 years ago
Depends on: 527181

Updated

8 years ago
Depends on: 527914
Blocks: 525823

Updated

8 years ago
Depends on: 527974

Updated

8 years ago
Depends on: 528761
Depends on: 529211

Updated

8 years ago
Depends on: 538487
Depends on: 520579
Duplicate of this bug: 544271

Updated

8 years ago
Depends on: 544924

Updated

8 years ago
Depends on: 545077
(Assignee)

Updated

7 years ago
Depends on: 551597

Updated

7 years ago
Depends on: 551925

Updated

7 years ago
Depends on: 557823

Updated

7 years ago
Depends on: 527268

Updated

7 years ago
Depends on: 559613

Updated

7 years ago
Depends on: 528289

Updated

7 years ago
Depends on: 529067

Updated

7 years ago
Depends on: 550763

Updated

7 years ago
Blocks: 557924

Updated

7 years ago
No longer depends on: 557823

Updated

7 years ago
No longer depends on: 529067

Updated

7 years ago
Depends on: 562253

Updated

7 years ago
Depends on: 563337
(Assignee)

Updated

7 years ago
No longer depends on: 528289

Updated

7 years ago
Depends on: 578616
(Assignee)

Updated

7 years ago
Depends on: 580522

Updated

7 years ago
No longer depends on: 578616

Updated

7 years ago
Depends on: 591135

Updated

7 years ago
Depends on: 592451

Updated

7 years ago
Depends on: 616720
You need to log in before you can comment on or make changes to this bug.