Add tab hover card previews
Categories
(Firefox :: Tabbed Browser, enhancement)
Tracking
()
People
(Reporter: harrygaming260, Assigned: dwalker)
References
(Depends on 1 open bug)
Details
Attachments
(2 files, 4 obsolete files)
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)
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.
Updated•1 year ago
|
Assignee | ||
Comment 2•1 year ago
|
||
Assignee | ||
Comment 3•1 year ago
|
||
Assignee | ||
Comment 4•1 year ago
|
||
Updated•1 year ago
|
Updated•1 year ago
|
Assignee | ||
Comment 5•1 year ago
|
||
Updated•1 year ago
|
Updated•1 year ago
|
Assignee | ||
Comment 6•1 year ago
|
||
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Comment 8•1 year 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
Assignee | ||
Comment 9•1 year 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
Comment 10•1 year 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.
Assignee | ||
Comment 11•1 year 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.
Comment 12•1 year 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.
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Comment 13•1 year 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");
Comment 14•1 year ago
|
||
Comment 15•1 year 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
Assignee | ||
Updated•1 year ago
|
Comment 16•11 months ago
|
||
Comment 17•11 months ago
|
||
bugherder |
Updated•11 months ago
|
Comment 21•5 months ago
|
||
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
Updated•5 months ago
|
Updated•3 months ago
|
Description
•