1603796 - support tabs.goForward and tabs.goBack

Status: RESOLVED FIXED

(WebExtensions :: Compatibility, enhancement, P3)

Description

[Shane Caraveo (:mixedpuppy)]

Chrome 72 introduced two new tab APIs, goForward and goBack. We should consider these for chrome-compat.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

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.

Comment 3

[Atique Ahmed Ziad [:atiqueahmedziad]]

I want to work on it.

Comment 4

[Caitlin Neiman [:caitmuenster]]

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

[Tomislav Jovanovic :zombie]

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.

1. https://searchfox.org/mozilla-central/rev/3a0a8e27/browser/components/extensions/schemas/tabs.json#964-994
2. https://searchfox.org/mozilla-central/rev/3a0a8e27/browser/components/extensions/parent/ext-tabs.js#912-920
3. 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.

Comment 6

[Atique Ahmed Ziad [:atiqueahmedziad]]

Attachment: Bug 1603796 - support tabs.goForward and tabs.goBack

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4fd31338db03
support tabs.goForward and tabs.goBack r=zombie

Comment 8

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 4fd31338db03 (Bug 1603796) for mochitest failures at test_ext_all_apis.html.
https://hg.mozilla.org/integration/autoland/rev/3c35ee4a1d14c869f5d64d1eaa70196f6fb7d796

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=297388677&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=4fd31338db03e798709e886ad827d956f4bf91d0

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

Comment 9

[Atique Ahmed Ziad [:atiqueahmedziad]]

@zombie, what could be the possible reason of this test failing ?
I am getting the test passing locally. Did I miss anything ?

Comment 10

[Rob Wu [:robwu]]

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 11

[Tomislav Jovanovic :zombie]

Thanks Rob.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd832923a26f
support tabs.goForward and tabs.goBack r=zombie

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 14

[Richard Bloor]

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

[Tomislav Jovanovic :zombie]

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

[Sebastian Zartner [:sebo]]

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

Comment 17

[Richard Bloor]

Thanks for the feedback and updates.