Closed Bug 1783521 Opened 2 years ago Closed 4 months ago

Add tab hover card previews

Categories

(Firefox :: Tabbed Browser, enhancement)

Product:

Component:

Version:

Firefox 103

Platform:

Desktop

Windows

Type:

enhancement

Priority:

Not set

Severity:

--

Tracking

()

Status:

RESOLVED FIXED

Milestone:

123 Branch

Tracking Flags:

Tracking

Status

firefox123

---

fixed

People

(Reporter: harrygaming260, Assigned: dwalker)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

chrome tab hover card preview.gif 2 years ago sea 3.55 MB, image/gif		Details
WIP: Bug 1783521 - add card preview when inactive tab hovered 10 months ago DJ (:dwalker) 48 bytes, text/x-phabricator-request		Details \| Review
WIP: Bug 1783521 - add card preview when inactive tab hoveredDifferential Revision: https://phabricator.services.mozilla.com/D183774 10 months ago DJ (:dwalker) 48 bytes, text/x-phabricator-request		Details \| Review
WIP: Bug 1783521 - add card preview when inactive tab hovered 10 months ago DJ (:dwalker) 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1783521 - add card preview when inactive tab hovered. r?mhowell,gijs 10 months ago DJ (:dwalker) 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1783521 - add card preview when inactive tab hovered. r?mhowell,gijs 10 months ago DJ (:dwalker) 48 bytes, text/x-phabricator-request		Details \| Review

Reporter

Description

•

2 years ago

Attached image chrome tab hover card preview.gif — Details

Steps to reproduce:

Start Firefox through Windows Search or by clicking the Firefox shortcut
Go to a website if not already in a website
Open another tab through the "Open a new tab" button in the tablist
Go to a website if not already in a website
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)

Reporter

Updated

•

2 years ago

Component: Untriaged → Tabbed Browser

OS: Unspecified → Windows

Hardware: Unspecified → Desktop

Hani Yacoub, Desktop QA

Comment 1

•

2 years ago

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

Molly Howell (she/her) (no longer active)

Updated

•

10 months ago

Assignee: nobody → dwalker

Status: NEW → ASSIGNED

See Also: → https://mozilla-hub.atlassian.net/browse/FIDEFE-3760

Assignee

Comment 2

•

10 months ago

Attached file WIP: Bug 1783521 - add card preview when inactive tab hovered (obsolete) — Details

Assignee

Comment 3

•

10 months ago

Attached file WIP: Bug 1783521 - add card preview when inactive tab hoveredDifferential Revision: https://phabricator.services.mozilla.com/D183774 (obsolete) — Details

Assignee

Comment 4

•

10 months ago

Attached file WIP: Bug 1783521 - add card preview when inactive tab hovered (obsolete) — Details

Phabricator Automation

Updated

•

10 months ago

Attachment #9344270 - Attachment is obsolete: true

Phabricator Automation

Updated

•

10 months ago

Attachment #9344246 - Attachment is obsolete: true

Assignee

Comment 5

•

10 months ago

Attached file Bug 1783521 - add card preview when inactive tab hovered. r?mhowell,gijs — Details

Phabricator Automation

Updated

•

10 months ago

Attachment #9344441 - Attachment is obsolete: true

Phabricator Automation

Updated

•

10 months ago

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

Assignee

Comment 6

•

10 months ago

Attached file Bug 1783521 - add card preview when inactive tab hovered. r?mhowell,gijs (obsolete) — Details

Phabricator Automation

Updated

•

10 months ago

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

Phabricator Automation

Updated

•

10 months ago

Attachment #9345703 - Attachment is obsolete: true

Giorgia Nichita, Release Desktop QA

Updated

•

9 months ago

Blocks: 1848157

Comment 7

•

8 months ago

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

Comment 8

•

8 months ago

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)

Assignee

Comment 9

•

8 months ago

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

re-running the job yields passing tests: treeherder link
tracking bug for intermittent failure: https://bugzilla.mozilla.org/show_bug.cgi?id=1783436

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

Comment 10

•

8 months ago

•

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)

Assignee

Comment 11

•

8 months ago

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)

BugBot [:suhaib / :marco/ :calixte]

Comment 12

•

8 months ago

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)

Assignee

Updated

•

8 months ago

Flags: needinfo?(dwalker)

Assignee

Updated

•

8 months ago

Flags: needinfo?(dao+bmo)

Mike Conley (:mconley) (:⚙️)

Comment 13

•

5 months ago

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");

Assignee

Updated

•

5 months ago

See Also: → 1869385

Comment 14

•

5 months ago

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

Narcis Beleuzu [:NarcisB]

Comment 15

•

5 months ago

Backed out for marionette failure on test_restore_manually.py

Backout link: https://hg.mozilla.org/integration/autoland/rev/ede008f65ded6409ee06a55d2b43c6b8c42cc852
Log link: https://treeherder.mozilla.org/logviewer?job_id=440907480&repo=autoland&lineNumber=95893

Flags: needinfo?(dwalker)

Assignee

Updated

•

5 months ago

Flags: needinfo?(dwalker)

Comment 16

•

4 months ago

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

Comment 17

•

4 months ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/a2601b26e44d

Status: ASSIGNED → RESOLVED

Closed: 4 months ago

status-firefox123: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → 123 Branch

Updated

•

4 months ago

Depends on: 1869385

See Also: 1869385 →

Updated

•

4 months ago

Depends on: 1875831

Nicolas Chevobbe [:nchevobbe][PTO - back on Monday May 13th]

Updated

•

3 months ago

Regressions: 1877088

Updated

•

3 months ago

Depends on: 1877837

Updated

•

3 months ago

Depends on: 1877838

Mike Conley (:mconley) (:⚙️)

Updated

•

3 months ago

Duplicate of this bug: 1738402

Chris Peterson [:cpeterson]

Updated

•

3 months ago

Depends on: 1881520

Emilio Cobos Álvarez (:emilio)

Updated

•

3 months ago

Blocks: 1881488

Updated

•

1 month ago

Depends on: 1888919

Emilio Cobos Álvarez (:emilio)

Updated

•

1 month ago

Depends on: 1890397

Emilio Cobos Álvarez (:emilio)

Updated

•

1 month ago

Depends on: 1890399

Emilio Cobos Álvarez (:emilio)

Updated

•

1 month ago

Depends on: 1891210

Tarek El-Eter [:Tarek]

Updated

•

1 month ago

Depends on: 1891228

Kelly Cochrane [:kcochrane]

Updated

•

27 days ago

Regressions: 1882114

Giorgia Nichita, Release Desktop QA

Updated

•

26 days ago

Duplicate of this bug: 1889916

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