support tabs.goForward and tabs.goBack
Categories
(WebExtensions :: Compatibility, enhancement, P3)
Tracking
(firefox77 fixed)
Tracking | Status | |
---|---|---|
firefox77 | --- | fixed |
People
(Reporter: mixedpuppy, Assigned: atiqueahmedziad, Mentored)
References
()
Details
(Keywords: dev-doc-complete, good-first-bug, parity-chrome)
Attachments
(1 file)
Chrome 72 introduced two new tab APIs, goForward and goBack. We should consider these for chrome-compat.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 1•5 years ago
|
||
The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.
Updated•5 years ago
|
Comment 2•5 years ago
|
||
I agree we should implement this, it seems fairly simple. This seems to be a good first bug, if someone from the team wants to take it beforehand that works as well.
Assignee | ||
Comment 3•5 years ago
|
||
I want to work on it.
Comment 4•5 years ago
|
||
If this is your first contribution, please refer to https://wiki.mozilla.org/Add-ons/Contribute/Code on how to get started.
Mentor: :zombie
Comment 5•5 years ago
|
||
Hi Atique, that's sounds great.
Here are some pointers to code that implements the tabs.reload()
method, which I expect to be very similar to these two new ones.
- https://searchfox.org/mozilla-central/rev/3a0a8e27/browser/components/extensions/schemas/tabs.json#964-994
- https://searchfox.org/mozilla-central/rev/3a0a8e27/browser/components/extensions/parent/ext-tabs.js#912-920
- https://searchfox.org/mozilla-central/rev/3a0a8e27/browser/components/extensions/test/browser/browser_ext_tabs_reload.js
I expect you'll need to 1) add method definitions to the schema, 2) which would need to call goBack()
and goForward()
on the tab's linkedBrowser
, 3) and of course some tests.
Let me know if you have any questions.
Assignee | ||
Comment 6•5 years ago
|
||
Updated•5 years ago
|
Comment 8•5 years ago
|
||
Backed out changeset 4fd31338db03 (Bug 1603796) for mochitest failures at test_ext_all_apis.html.
https://hg.mozilla.org/integration/autoland/rev/3c35ee4a1d14c869f5d64d1eaa70196f6fb7d796
Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=297388677&repo=autoland&lineNumber=1633
and there are also some TV failures {tier 2 atm] on browser_ext_tabs_goBack_goForward.js | tab1 is found as expected - Expected: tab1, Actual: New Tab.
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=297383510&repo=autoland&lineNumber=1347
Assignee | ||
Comment 9•5 years ago
|
||
@zombie, what could be the possible reason of this test failing ?
I am getting the test passing locally. Did I miss anything ?
Comment 10•5 years ago
|
||
test_ext_all_apis.html
should consistently fail, and also locally. The test exists to discourage the (accidental) introduction of new APIs without permissions. Since this new API is not very dangerous, you can get the test to pass by adding the two new methods to the (alphabetically sorted) list at https://searchfox.org/mozilla-central/rev/2cd2d511c0d94a34fb7fa3b746f54170ee759e35/browser/components/extensions/test/mochitest/test_ext_all_apis.html#17,35
The browser_ext_tabs_goBack_goForward.js
seems to be an intermittent failure. Locally run the test with --verify
to try and reproduce it (this corresponds to the TV job in the treeherder link in comment 8). To help with the investigation, consider temporarily logging the values of tabId
, changeInfo
and tabInfo
with the dump
function.
P.S.: This API doesn't seem to have hard dependencies on platform/UI-specific logic. Therefore it may also be a good idea to also add the API to the Android implementation (either in this bug or in a follow-up bug). To implement this on Android, you need to add the APIs to mobile/android/components/extensions/test/mochitest/test_ext_all_apis.html and the implementation (schemas/tabs.json + ext-tabs.js) + mochitest in https://searchfox.org/mozilla-central/source/mobile/android/components/extensions
Comment 12•5 years ago
|
||
Comment 13•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Comment 14•4 years ago
•
|
||
The following documentation has been updated:
- compatibility data, changes submitted in 6173
- details added to the release notes
- pages for goForward and goBack added, however, additional information is needed as follows:
-
- the schema file included no description for the callback, what do we need to say about this?
-
- what does the promise return?
-
- are the code examples appropriate?
I'll update the API references on your response, please let me know if any of the other documentation needs changing. Thank you
Comment 15•4 years ago
|
||
Thanks Richard, the page and examples looks good.
There isn't much to say about the callback/returned promise, they just get called/resolved (with nothing) when the operation is done.
In general, any Chrome-compatible API will have a callback (with or without some params), which we transpose into a promise (resolved with the same params as a value, or nothing).
In this case, looking up the docs of Chrome's implementation says as much: https://developer.chrome.com/extensions/tabs#method-goForward
Comment 16•4 years ago
|
||
The descriptions also look good to me. I've added one for the callback and the returned promise according to :zombie's feedback, and also added some tags to the pages.
The compatibility data needs to be checked by the MDN team, though to me it seems correct.
Sebastian
Updated•4 years ago
|
Comment 17•4 years ago
|
||
Thanks for the feedback and updates.
Description
•