Closed Bug 1603796 Opened 4 years ago Closed 4 years ago

support tabs.goForward and tabs.goBack

Categories

(WebExtensions :: Compatibility, enhancement, P3)

enhancement

Tracking

(firefox77 fixed)

RESOLVED FIXED
mozilla77
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.

Flags: needinfo?(philipp)
Priority: -- → P3
Component: General → Compatibility

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

Priority: P3 → --
Priority: -- → P3

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.

Flags: needinfo?(philipp)
Keywords: good-first-bug

I want to work on it.

If this is your first contribution, please refer to https://wiki.mozilla.org/Add-ons/Contribute/Code on how to get started.

Mentor: :zombie

Mentor: tomica

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.

Assignee: nobody → softfilebd
Status: NEW → ASSIGNED
Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4fd31338db03
support tabs.goForward and tabs.goBack r=zombie

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

Flags: needinfo?(softfilebd) → needinfo?(tomica)

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

Thanks Rob.

Flags: needinfo?(tomica)
Pushed by tjovanovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dd832923a26f
support tabs.goForward and tabs.goBack r=zombie
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77

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

Flags: needinfo?(tomica)
Flags: needinfo?(sebastianzartner)

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

Flags: needinfo?(tomica)

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

Flags: needinfo?(sebastianzartner)

Thanks for the feedback and updates.

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

Attachment

General

Created:
Updated:
Size: