WebExtension bookmarks context menu items should appear in bookmarks sidebar and library window
Categories
(WebExtensions :: General, defect, P5)
Tracking
(firefox66 fixed)
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: ntim, Assigned: robwu)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(4 files, 2 obsolete files)
Updated•7 years ago
|
Updated•6 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•6 years ago
|
Updated•6 years ago
|
Comment 4•6 years ago
|
||
Comment 5•6 years ago
|
||
mozreview-review |
Comment 6•6 years ago
|
||
mozreview-review |
Comment 7•6 years ago
|
||
Comment 8•6 years ago
|
||
Comment 9•6 years ago
|
||
Comment 10•6 years ago
|
||
Comment 11•6 years ago
|
||
Assignee | ||
Comment 12•6 years ago
|
||
Assignee | ||
Comment 13•6 years ago
|
||
Assignee | ||
Comment 14•6 years ago
|
||
Reporter | ||
Comment 15•6 years ago
|
||
Comment 16•6 years ago
|
||
Assignee | ||
Comment 17•6 years ago
|
||
Comment 18•6 years ago
|
||
Assignee | ||
Comment 19•6 years ago
|
||
Reporter | ||
Comment 20•6 years ago
|
||
Assignee | ||
Comment 21•6 years ago
|
||
Assignee | ||
Comment 22•6 years ago
|
||
Assignee | ||
Comment 23•6 years ago
|
||
Assignee | ||
Comment 24•6 years ago
|
||
Assignee | ||
Comment 25•6 years ago
|
||
I've moved Peter's patches to Phabricator, rebased on the latest checkout, without modifications.
I'll then apply my new (minimal) changes, to make review easier.
Assignee | ||
Updated•6 years ago
|
Comment 26•6 years ago
|
||
Comment 27•6 years ago
|
||
Backed out changeset ee53bdc5b1d4 (Bug 1419195) for failing in browser_ext_contextMenus.js
Backed out changeset 732184f122e3 (Bug 1419195) for failing in browser_ext_contextMenus.js
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=221752920&repo=autoland&lineNumber=16633
Backout: https://hg.mozilla.org/integration/autoland/rev/6573f476d87a23ad9a25413587df39e1ef0fb483
https://hg.mozilla.org/integration/autoland/rev/eb00c4fbfb23b2382a2e76bc2985ee947e489163
Comment 28•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 29•6 years ago
|
||
The patch includes a new test that triggers the failures of comment 27, but the cause is pre-existing. I can reproduce the leaks without any extension code - see bug 1520047.
I'm going to reland the patches with the test being disabled on debug + TV builds.
Comment 30•6 years ago
|
||
Assignee | ||
Comment 31•6 years ago
|
||
Patches are relanding, with one specific part of a test task disabled due to bug 1520047 .
Green try run is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ff4d4b1c3ba2c566c31cc76793b141bd2c601d9
Comment 32•6 years ago
|
||
Backed out 3 changesets (bug 1515810, bug 1419195) for failing at /browser/browser_ext_contextMenus.js on a CLOSED TREE
Backout link: https://hg.mozilla.org/integration/autoland/rev/65b6d0b670b410489153b2086d8dc1a1b66a3231
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=437003de9fffafd6656b79ea4bcbeab0aa652eef
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=221984836&repo=autoland&lineNumber=4290
Log snippet:
06:40:03 INFO - Extension loaded
06:40:03 INFO - Console message: Warning: attempting to write 7793 bytes to preference extensions.webextensions.uuids. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes.
06:40:03 INFO - Buffered messages logged at 06:38:49
06:40:03 INFO - Console message: [JavaScript Error: "TypeError: tree.boxObject.getCellAt is not a function" {file: "chrome://browser/content/parent/ext-menus.js" line: 1044}]
06:40:03 INFO - Buffered messages finished
06:40:03 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_contextMenus.js | Test timed out -
06:40:03 INFO - Not taking screenshot here: see the one that was previously logged
06:40:03 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_contextMenus.js | no tasks awaiting on messages - Got ["test-finish"], expected []
06:40:03 INFO - Stack trace:
06:40:03 INFO - chrome://mochikit/content/browser-test.js:test_is:1318
06:40:03 INFO - chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:ExtensionTestUtils.loadExtension/<:31
06:40:03 INFO - chrome://mochikit/content/browser-test.js:nextTest:707
06:40:03 INFO - chrome://mochikit/content/browser-test.js:timeoutFn:1205
06:40:03 INFO - setTimeout handlerchrome://mochikit/content/browser-test.js:Tester_execTest:1167
06:40:03 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:997
06:40:03 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
06:40:03 INFO - Not taking screenshot here: see the one that was previously logged
06:40:03 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_contextMenus.js | Extension left running at test shutdown -
06:40:03 INFO - Stack trace:
06:40:03 INFO - chrome://mochikit/content/browser-test.js:test_ok:1307
06:40:03 INFO - chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:ExtensionTestUtils.loadExtension/<:109
06:40:03 INFO - chrome://mochikit/content/browser-test.js:nextTest:707
06:40:03 INFO - chrome://mochikit/content/browser-test.js:timeoutFn:1205
06:40:03 INFO - setTimeout handlerchrome://mochikit/content/browser-test.js:Tester_execTest:1167
06:40:03 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:997
06:40:03 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
06:40:03 INFO - GECKO(835) | MEMORY STAT | vsize 4637MB | residentFast 614MB | heapAllocated 99MB
06:40:03 INFO - TEST-OK | browser/components/extensions/test/browser/browser_ext_contextMenus.js | took 90139ms
06:40:03 INFO - Not taking screenshot here: see the one that was previously logged
06:40:03 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_contextMenus.js | Found a tab after previous test timed out: http://mochi.test:8888/browser/browser/components/extensions/test/browser/context.html -
06:40:03 INFO - GECKO(835) | ++DOCSHELL 0x10b91f800 == 1 [pid = 844] [id = {c8913f40-b046-034d-afa7-d2c731357112}]
06:40:03 INFO - GECKO(835) | ++DOMWINDOW == 1 (0x10b987c00) [pid = 844] [serial = 40] [outer = 0x0]
06:40:03 INFO - GECKO(835) | ++DOMWINDOW == 2 (0x10b9ebc00) [pid = 844] [serial = 41] [outer = 0x10b987c00]
06:40:03 INFO - GECKO(835) | ++DOMWINDOW == 3 (0x11afcdc00) [pid = 844] [serial = 42] [outer = 0x10b987c00]
06:40:03 INFO - checking window state
Assignee | ||
Comment 33•6 years ago
|
||
The failure is caused by the refactor in bug 1482389. Patches for that bug landed between my try push and autoland.
I'm rebasing the patch and will reland if try is green again.
Comment 34•6 years ago
|
||
Comment 35•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6b7a9a7afa56
https://hg.mozilla.org/mozilla-central/rev/8b35181c3ccc
Comment 36•6 years ago
|
||
The docs[1] also needs to be updated.
[1] https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/ContextType#Type
Assignee | ||
Comment 37•6 years ago
|
||
Thanks kernp25. This should also be documented in the Add-ons section of the Firefox 66 for developers article @ https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/66#Changes_for_add-on_developers
Comment 38•6 years ago
|
||
Will this bug require manual validation from the QA team? I've looked over the description but not sure where exactly the context menu should appear for the sidebar and library, could you please provide more details? Thanks.
Assignee | ||
Comment 39•6 years ago
|
||
Sidebar: Ctrl-B to open bookmark sidebar, expand a bookmark folder and right-click on a bookmark.
Library: Ctrl-Shift-O (or Bookmarks > Show All Bookmarks), right-click on a bookmark.
To test, you could use any add-on that registers a bookmark menu item, e.g. https://addons.mozilla.org/en-US/firefox/addon/open-bookmark-in-container-tab/
The feature has automated tests, and I just verified on Nightly that the menu appears as expected, so qe-verify-.
Comment 40•6 years ago
|
||
Added the following paragraph to the context menus page:
In Firefox 66 and later, context menus defined by a an extension can also appear in the Bookmarks Sidebar (Ctrl + B) or the Library window (Ctrl + Shift + B). For example, the extension "Open bookmark in Container Tab" allows the user to open a bookmark URL in a new container tab
I have included the GitHub link for the extension as requested in IRC and added an image showing the Open in New Container Tab context menu opened over the Bookmarks sidebar.
I also added this to the Firefox 66 release notes under API changes/Menus:
"Context menus added by an extension will appear in the Bookmarks sidebar (Ctrl + B) or Library window (Ctrl + Shift + B) (bug 1419195)."
Let me know if this is enough.
Assignee | ||
Comment 41•6 years ago
|
||
Thanks for the pararaph and screenshot at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/user_interface/Context_menu_items
I moved the version-specific information to the "bookmarks" type at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/ContextType (here is the difference), and reworded the paragraph at the "Context menu items" tutorial to be a bit more generic (and reference the ContextType article for further reference).
I also tweaked the release notes to add a reference to the ContextType article to improve its discoverability:
"Extension menu items of the "bookmark" type will also appear in the Bookmarks sidebar (Ctrl + B) and Library window (Ctrl + Shift + B) (bug 1419195)."
Assignee | ||
Updated•6 years ago
|
Comment 42•6 years ago
|
||
(In reply to Rob Wu [:robwu] from comment #41)
Thanks for the pararaph and screenshot at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/user_interface/Context_menu_items
I moved the version-specific information to the "bookmarks" type at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/ContextType (here is the difference), and reworded the paragraph at the "Context menu items" tutorial to be a bit more generic (and reference the ContextType article for further reference).
I also tweaked the release notes to add a reference to the ContextType article to improve its discoverability:
"Extension menu items of the "bookmark" type will also appear in the Bookmarks sidebar (Ctrl + B) and Library window (Ctrl + Shift + B) (bug 1419195)."
Thank you!
Assignee | ||
Updated•2 years ago
|
Description
•