Closed Bug 1783521 Opened 2 years ago Closed 11 months ago

Add tab hover card previews

Categories

(Firefox :: Tabbed Browser, enhancement)

Firefox 103
Desktop
Windows
enhancement

Tracking

()

RESOLVED FIXED
129 Branch
Tracking Status
relnote-firefox --- 129+
firefox123 --- fixed

People

(Reporter: harrygaming260, Assigned: dwalker)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

Steps to reproduce:

  1. Start Firefox through Windows Search or by clicking the Firefox shortcut
  2. Go to a website if not already in a website
  3. Open another tab through the "Open a new tab" button in the tablist
  4. Go to a website if not already in a website
  5. Hover over the inactive tab in the tablist until a tooltip appears

Actual results:

Tooltip appears under the cursor and shows the tab name without a tab hover card preview like web browsers (e.g. Google Chrome, Vivaldi) or pre-quantum extensions (e.g. TabScope)

Expected results:

Tooltip appears under the tab and shows the tab name with a tab hover card preview like web browsers (e.g. Google Chrome, Vivaldi [check example in attached file]) or pre-quantum extensions (e.g. TabScope)

Component: Untriaged → Tabbed Browser
OS: Unspecified → Windows
Hardware: Unspecified → Desktop

I will set this enhancement as new so the engineering team could decide if they take in consideration changing this.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → dwalker
Status: NEW → ASSIGNED
Attachment #9344270 - Attachment is obsolete: true
Attachment #9344246 - Attachment is obsolete: true
Attachment #9344441 - Attachment is obsolete: true
Attachment #9345123 - Attachment description: WIP: Bug 1783521 - add card preview when inactive tab hovered. r?mhowell → Bug 1783521 - add card preview when inactive tab hovered. r?mhowell
Attachment #9345123 - Attachment description: Bug 1783521 - add card preview when inactive tab hovered. r?mhowell → Bug 1783521 - add card preview when inactive tab hovered. r?mhowell,gijs
Attachment #9345703 - Attachment is obsolete: true
Pushed by dwalker@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5f5354d30a45 add card preview when inactive tab hovered. r=settings-reviewers,desktop-theme-reviewers,tabbrowser-reviewers,fluent-reviewers,flod,dao,mconley

Backed out for causing bc failures on browser_ext_windows_create_url.js.

[task 2023-09-08T06:53:32.680Z] 06:53:32     INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_windows_create_url.js | Dialog element should exist - [object XULElement] == true - 
[task 2023-09-08T06:53:32.681Z] 06:53:32     INFO - Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use <!DOCTYPE html>." {file: "moz-extension://19f385de-1e3f-458d-bc6c-3c62a1af258e/page.html" line: 0}]
[task 2023-09-08T06:53:32.682Z] 06:53:32     INFO - Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use <!DOCTYPE html>." {file: "moz-extension://19f385de-1e3f-458d-bc6c-3c62a1af258e/page.html?val=ext%2Bfoo%3Abar" line: 0}]
[task 2023-09-08T06:53:32.683Z] 06:53:32     INFO - resolving a window load
[task 2023-09-08T06:53:32.683Z] 06:53:32     INFO - resolving a window load
[task 2023-09-08T06:53:32.683Z] 06:53:32     INFO - resolving a window load
[task 2023-09-08T06:53:32.684Z] 06:53:32     INFO - resolving a window load
[task 2023-09-08T06:53:32.684Z] 06:53:32     INFO - resolving a window load
[task 2023-09-08T06:53:32.684Z] 06:53:32     INFO - Buffered messages finished
[task 2023-09-08T06:53:32.685Z] 06:53:32     INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_windows_create_url.js | Test timed out - 
[task 2023-09-08T06:53:32.685Z] 06:53:32     INFO - Not taking screenshot here: see the one that was previously logged
[task 2023-09-08T06:53:32.686Z] 06:53:32     INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_windows_create_url.js | Extension left running at test shutdown - 
[task 2023-09-08T06:53:32.686Z] 06:53:32     INFO - Stack trace:
[task 2023-09-08T06:53:32.686Z] 06:53:32     INFO - chrome://mochikit/content/browser-test.js:test_ok:1580
[task 2023-09-08T06:53:32.686Z] 06:53:32     INFO - chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:ExtensionTestUtils.loadExtension/<:132
[task 2023-09-08T06:53:32.686Z] 06:53:32     INFO - chrome://mochikit/content/browser-test.js:nextTest:702
[task 2023-09-08T06:53:32.686Z] 06:53:32     INFO - chrome://mochikit/content/browser-test.js:timeoutFn:1437
[task 2023-09-08T06:53:32.686Z] 06:53:32     INFO - setTimeout handler*chrome://mochikit/content/browser-test.js:Tester_execTest:1379
[task 2023-09-08T06:53:32.686Z] 06:53:32     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1120
[task 2023-09-08T06:53:32.686Z] 06:53:32     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1056
[task 2023-09-08T06:53:32.687Z] 06:53:32     INFO - Not taking screenshot here: see the one that was previously logged
[task 2023-09-08T06:53:32.688Z] 06:53:32     INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_windows_create_url.js | Extension left running at test shutdown - 
[task 2023-09-08T06:53:32.688Z] 06:53:32     INFO - Stack trace:
[task 2023-09-08T06:53:32.688Z] 06:53:32     INFO - chrome://mochikit/content/browser-test.js:test_ok:1580
[task 2023-09-08T06:53:32.688Z] 06:53:32     INFO - chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:ExtensionTestUtils.loadExtension/<:132
[task 2023-09-08T06:53:32.688Z] 06:53:32     INFO - chrome://mochikit/content/browser-test.js:nextTest:702
[task 2023-09-08T06:53:32.688Z] 06:53:32     INFO - chrome://mochikit/content/browser-test.js:timeoutFn:1437
[task 2023-09-08T06:53:32.688Z] 06:53:32     INFO - setTimeout handler*chrome://mochikit/content/browser-test.js:Tester_execTest:1379
[task 2023-09-08T06:53:32.688Z] 06:53:32     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1120
[task 2023-09-08T06:53:32.689Z] 06:53:32     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1056
[task 2023-09-08T06:53:32.689Z] 06:53:32     INFO - GECKO(3036) | MEMORY STAT | vsize 1539MB | vsizeMaxContiguous 1293MB | residentFast 655MB | heapAllocated 268MB
[task 2023-09-08T06:53:32.689Z] 06:53:32     INFO - TEST-OK | browser/components/extensions/test/browser/browser_ext_windows_create_url.js | took 48267ms
Flags: needinfo?(dwalker)

It looks like that test has historically caused intermittent failures and is continuing to do so.

Flags: needinfo?(dwalker) → needinfo?(imoraru)

This was permafailing. The retiggers that you did are not running the test that is failing - access the log for a green retrigger and search for browser/components/extensions/test/browser/browser_ext_windows_create_url.js - you will see that the job did not run the test.
In the link you provided, you will see a job that failed, this one , this ran the test and it failed, that is how I found that this was causing permafailures.

Flags: needinfo?(imoraru) → needinfo?(dwalker)

Thanks for the info Iulian, apologies for my error. I'll have to do some debugging to see what's going on with that test.

Flags: needinfo?(dwalker)

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:dwalker, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Flags: needinfo?(dwalker)
Flags: needinfo?(dao+bmo)
Flags: needinfo?(dwalker)
Flags: needinfo?(dao+bmo)

DJ and I looked at this together today, and I think I know what's going on.

The thing about browser_ext_windows_create_url.js, is that it creates several windows in parallel, and then collects data from each window before the test can exit.

Part of what these tests do is wait for the browser-delayed-startup-finished observer notification, and sets up an event listener for any opened WebExtension permission dialogs to ensure that they get accepted.

The problem is that browser-delayed-startup-finished only fires at the end of gBrowserInit._delayedStartup, and _delayedStartup is only ever invoked after the first paint of the browser window occurs.

So we have a race! If a window opens, but it never paints (perhaps because another newly opened window occluded it!), then browser-delayed-startup-finished will not fire for those windows, and if those windows happen to be ones that open dialogs, those dialogs will not be accepted, and so the test will halt, waiting for those things to occur.

I think this was always a race, but DJ's patch exacerbated it by adding a new <script type="module">. Module scripts load off of the main thread, but DOMContentLoaded / load (which is what sets up the MozAfterPaint event listener in browser.js) will not fire until the module has finished loading.

I think we can modify this test to not be so race-y. We can wait for each window to be created, and foregrounded one at a time, before moving onto the next. Here's the patch I suggest:

diff --git a/browser/components/extensions/test/browser/browser_ext_windows_create_url.js b/browser/components/extensions/test/browser/browser_ext_windows_create_url.js
--- a/browser/components/extensions/test/browser/browser_ext_windows_create_url.js
+++ b/browser/components/extensions/test/browser/browser_ext_windows_create_url.js
@@ -159,20 +159,42 @@ add_task(async function testWindowCreate
       } finally {
         // Close opened windows, whether they were expected or not.
         if (window) {
           await browser.windows.remove(window.id);
         }
       }
     }
     try {
-      // First collect all results, in parallel.
-      const results = await Promise.allSettled(
-        TEST_SETS.map(t => create({ url: t.url }))
-      );
+      // First collect all results. We wait for each window to create before
+      // moving on to the next one to avoid cases where delayed startup might
+      // not fire for a window because it is backgrounded / fully occluded
+      // before it can paint.
+      let promises = [];
+      for (let testCase of TEST_SETS) {
+        try {
+          let resultPromise = create({ url: testCase.url });
+          // We will be examining the results of each call to
+          // create in checkCreateResult after converting them
+          // to a set of objects describing their resolution state
+          // via allSettled. Still, to avoid weird races involving
+          // window paints when opening many windows at once, we'll
+          // await each window creation 
+          promises.push(resultPromise);
+          await resultPromise;
+        } catch(e) {
+          // We expect some of the create calls to fail.
+        }
+      }
+
+      // Convert each Promise into something that describes how each of
+      // them either resolved or rejected. That's something we get for
+      // free from allSettled.
+      const results = await Promise.allSettled(promises);
+
       // Then check the results sequentially
       await Promise.all(
         TEST_SETS.map((t, i) => checkCreateResult(results[i], t))
       );
       browser.test.notifyPass("window-create-url");
     } catch (e) {
       browser.test.fail(`${e} :: ${e.stack}`);
       browser.test.notifyFail("window-create-url");
See Also: → 1869385
Pushed by dwalker@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fe2ffeac3ec2 add card preview when inactive tab hovered. r=settings-reviewers,desktop-theme-reviewers,tabbrowser-reviewers,fluent-reviewers,flod,dao,mconley
Flags: needinfo?(dwalker)
Pushed by dwalker@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a2601b26e44d add card preview when inactive tab hovered. r=settings-reviewers,desktop-theme-reviewers,tabbrowser-reviewers,fluent-reviewers,flod,dao,mconley
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → 123 Branch
Depends on: 1869385
See Also: 1869385
Depends on: 1875831
Regressions: 1877088
Depends on: 1877837
Depends on: 1877838
Duplicate of this bug: 1738402
Depends on: 1881520
Depends on: 1888919
Depends on: 1890397
Depends on: 1890399
Depends on: 1891210
Depends on: 1891228
Regressions: 1882114
Duplicate of this bug: 1889916
Duplicate of this bug: 1619054
Depends on: 1876582

Release Note Request (optional, but appreciated)
[Why is this notable]: This bug (to add the tab hover preview pref, disabled by default) was resolved in Fx 123. We now plan to ship it to Release in Fx 129 (via a Nimbus flag set to 100% rollout), so I'd like to mention this feature in Fx 129's release notes.
[Affects Firefox for Android]: No
[Suggested wording]: Mouse over your background tabs to quickly glance at a page preview without switching tabs. Easily locate the tab you need among many open tabs.
[Links (documentation, blog post, etc)]: None

relnote-firefox: --- → ?
Target Milestone: 123 Branch → 129 Branch
No longer blocks: tab-preview
Depends on: tab-preview
Depends on: 1908566
Depends on: 1915009
Depends on: 1920353
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: