New tab and restored tab notified via tabs.onCreated can have invalid (too large) index

VERIFIED FIXED in Firefox 68

Status

defect
P3
normal
VERIFIED FIXED
2 months ago
14 days ago

People

(Reporter: yuki, Assigned: yuki)

Tracking

Trunk
mozilla68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 verified)

Details

Attachments

(2 attachments)

Assignee

Description

2 months ago

Firefox notifies new tab or restored tab to listeners of tabs.onCreated, but the notified tab can have invalid index which is different from actual index on the tab bar.

Steps to reproduce

  1. Register an listener to tabs.onCreated, like:
    chrome.tabs.onCreated.addListener(tab => console.log(tab))
  2. Open new tab with too larget index, like:
    chrome.tabs.create({ index: 9999999 })
  3. Open four ore more tabs.
  4. Close all tabs opened at the step 3, with the reversed order they were opened.
    (In other words, close tabs from the last, to the first.)
  5. Open the history menu and restore the initially closed and last opened tab.

Actual result

Both tabs have their index larger than tabs.length - 1.
The tab opened at the step 2 has 9999999.
The tab restored at the step 5 has the index same to the one when it was closed.

Expected result

Both tabs have their index same to tabs.length - 1.

Environment

  • Nightly 68.0a1 on Ubuntu 16.04LTS
  • Firefox 66.0.2 on Ubuntu 16.04LTS

Additional information

Google Chrome (Chromium) works as expected. I've attached a testcase extension, and I got a console output like:

opened with index 2
there are 3 tabs
opened with index 2
there are 3 tabs

On the other hand, Firefox reports:

opened with index 9999999 <= larger than tabs.length-1
there are 3 tabs
opened with index 4 <= larger than tabs.length-1
there are 3 tabs

Comment 1

2 months ago

Is it a dupe of bug 1504775?

Flags: needinfo?(yuki)
Assignee

Comment 2

2 months ago

The bug 1504775 looked to cause this. But the patch contains changes only on the XUL layer, and there is no guarantee about compatibility with Chromeium's extensions API. One more automated test on the WebExtensions API layer looks better for more clear compatibility.

Flags: needinfo?(yuki)

Are you saying that this behavior started after the patch from bug 1504775? I don't see how that is logically possible.

Flags: needinfo?(yuki)
Assignee

Comment 4

a month ago

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)

Are you saying that this behavior started after the patch from bug 1504775?

No, I meant its opposite. I think that the patch coincidentally fixes this WebExtensions API compatibility bug, and a new test to verify the compatibility is required. Thus I'm trying to write a patch for future consistency. Is such a "only including test" patch acceptable?

Flags: needinfo?(yuki)
Regressed by: 1504775
Depends on: 1504775
No longer regressed by: 1504775
Priority: -- → P3
Assignee: nobody → yuki
Assignee

Updated

a month ago
Keywords: checkin-needed

Comment 6

a month ago

Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fd7d4206ad59
Add tests to verify WebExtensions API compatibility around new tab index r=mixedpuppy

Keywords: checkin-needed

Comment 7

a month ago
bugherder
Status: NEW → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Comment 8

14 days ago

This issue is verified as fixed on Firefox 68.0a1 (20190508111321) under Win 7 64-bit and Mac OS X 10.14.1
I have used the test add-on from the attachment and the issue is no longer reproducing. The following is displayed in the console:

opened with index 2
there are 3 tabs

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.