1416573 - bookmarks.create, index is not taking in account

Status: RESOLVED INVALID

(WebExtensions :: General, defect)

Description

[Morikko]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171106144225

Steps to reproduce:

I am bookmarking a list of tabs.

>> Code example:
var rootId;
// 1. Create root
browser.bookmarks.create({
  title: "Index Test"
}).then((bmRoot) => {
  rootId = bmRoot.id;
  browser.tabs.query({
    currentWindow: true
  }).then((tabs) => {
    tabs.map((tab) => {
      console.log("Creation: " + tab.index)
      browser.bookmarks.create({
        title: tab.title,
        index: tab.index,
        url: tab.url,
        parentId: rootId
      }).then((bookmark)=>{
        console.log(bookmark.title + " " + bookmark.index);
      });
    });
  });
});


Actual results:

Bookmarks are created in the right index order, however the resulting bookmarks are all with index 0. Thus, the last tabs appear first.

>> Output:
Creation: 0 
Creation: 1 
Creation: 2 
Creation: 3 
Creation: 4 
Creation: 5 
Creation: 6 
Problem to recieve message in Content Script - Add-ons / Development - Mozilla Discourse 0 
console.error() - Référence Web API | MDN 0 
javascript - Adjust width of input field to its input - Stack Overflow 0 
css - How do I set the size of an HTML text box? - Stack Overflow 0 
html - Input size vs width - Stack Overflow 0 
reactjs - How to add style to React Element if it's created this way? - Stack Overflow 0 
Debugging with Firefox Developer Tools 0


Expected results:

As I am giving the index in browser.bookmarks.create, bookmarks should be appended one after the other one. (And not one before the other one).

Comment 1

[Bob Silverberg [:bsilverberg]]

I will investigate.

Comment 2

[Bob Silverberg [:bsilverberg]]

This is not a problem with the bookmarks API, but a problem with your code. Bookmarks created via bookmarks.create() are created asynchronously, and in your code you do not wait for a bookmark to finish being created before you start the creation of the next one, so each of them end up with an index of 0. If you change your code to wait for a bookmark to be created before trying to create the next one you'll find that it works as expected. An example of changing your code to do this would be:

async function createBookmarks() {
  let bmRoot = await browser.bookmarks.create({
    title: "Index Test"
  });
  let rootId = bmRoot.id;
  let tabs = await browser.tabs.query({
    currentWindow: true
  });
  for (let tab of tabs) {
    console.log("Creation: " + tab.index)
    let bookmark = await browser.bookmarks.create({
      title: tab.title,
      index: tab.index,
      url: tab.url,
      parentId: rootId
    });
    console.log(bookmark.title + " " + bookmark.index);
  }
}

Comment 3

[Morikko]

Sorry for the disagreement: I got confused because the behavior with tabs.create is a bit different.

tabs.create does kind of "reserve the index", so that the next call even without waiting the tab to be completely created, creates a tab after the first one. 

Do you know if for tabs.create, I can always be sure index will be kept without waiting completely ?

I changed the behavior for tabs.create in my extension Sync Tab Groups for completely wait the creation. The results is that I can see each tab being open after the next one whereas without waiting they were opening at once. The visual style is worth, so I would prefer using the first behavior.

Example: 

// 3 tabs
tabs = '[{"id":247,"index":2,"windowId":838,"highlighted":true,"active":true,"pinned":false,"status":"complete","discarded":false,"incognito":false,"width":960,"height":943,"lastAccessed":1511263218462,"audible":false,"mutedInfo":{"muted":false},"url":"https://stackoverflow.com/questions/3392493/adjust-width-of-input-field-to-its-input","title":"javascript - Adjust width of input field to its input - Stack Overflow","favIconUrl":"https://cdn.sstatic.net/Sites/stackoverflow/img/favicon.ico?v=4f32ecc8f43d"},{"id":248,"index":3,"windowId":838,"highlighted":false,"active":false,"pinned":false,"status":"complete","discarded":false,"incognito":false,"width":960,"height":943,"lastAccessed":1511263211435,"audible":false,"mutedInfo":{"muted":false},"url":"https://www.computerhope.com/jargon/e/event-listener.htm","title":"What is an Event Listener?","favIconUrl":"https://www.computerhope.com/cdn/favicon.ico"},{"id":249,"index":4,"windowId":838,"highlighted":false,"active":false,"pinned":false,"status":"complete","discarded":false,"incognito":false,"width":960,"height":943,"lastAccessed":1511263211617,"audible":false,"mutedInfo":{"muted":false},"url":"https://www.google.fr/search?q=Immutable+from+JS&ie=utf-8&oe=utf-8&client=firefox-b&gfe_rd=cr&dcr=0&ei=xgkBWoOEPezBXr2sjKgN","title":"Immutable from JS - Recherche Google","favIconUrl":"https://www.google.fr/images/branding/product/ico/googleg_lodp.ico"}]';

tabs = JSON.parse(tabs)

// 1. Without waiting:
openTabs = async function( ) {
  let index = 0;
  for (let tab of GroupManager.groups[2].tabs ) {
    browser.tabs.create({
      url: tab.url,
      index: index
    });
    index++;
  }
}


// 2. Waiting behavior:
openWaitTabs = async function( ) {
  let index = 0;
  for (let tab of tabs ) {
    await browser.tabs.create({
      url: tab.url,
      index: index
    });
    index++;
  }
}

Comment 4

[Morikko]

Sorry, there was a mistake.

Example: 

// 3 tabs
tabs = '[{"id":247,"index":2,"windowId":838,"highlighted":true,"active":true,"pinned":false,"status":"complete","discarded":false,"incognito":false,"width":960,"height":943,"lastAccessed":1511263218462,"audible":false,"mutedInfo":{"muted":false},"url":"https://stackoverflow.com/questions/3392493/adjust-width-of-input-field-to-its-input","title":"javascript - Adjust width of input field to its input - Stack Overflow","favIconUrl":"https://cdn.sstatic.net/Sites/stackoverflow/img/favicon.ico?v=4f32ecc8f43d"},{"id":248,"index":3,"windowId":838,"highlighted":false,"active":false,"pinned":false,"status":"complete","discarded":false,"incognito":false,"width":960,"height":943,"lastAccessed":1511263211435,"audible":false,"mutedInfo":{"muted":false},"url":"https://www.computerhope.com/jargon/e/event-listener.htm","title":"What is an Event Listener?","favIconUrl":"https://www.computerhope.com/cdn/favicon.ico"},{"id":249,"index":4,"windowId":838,"highlighted":false,"active":false,"pinned":false,"status":"complete","discarded":false,"incognito":false,"width":960,"height":943,"lastAccessed":1511263211617,"audible":false,"mutedInfo":{"muted":false},"url":"https://www.google.fr/search?q=Immutable+from+JS&ie=utf-8&oe=utf-8&client=firefox-b&gfe_rd=cr&dcr=0&ei=xgkBWoOEPezBXr2sjKgN","title":"Immutable from JS - Recherche Google","favIconUrl":"https://www.google.fr/images/branding/product/ico/googleg_lodp.ico"}]';

tabs = JSON.parse(tabs)

// 1. Without waiting:
openTabs = async function( ) {
  let index = 0;
  for (let tab of tabs ) {
    browser.tabs.create({
      url: tab.url,
      index: index
    });
    index++;
  }
}


// 2. Waiting behavior:
openWaitTabs = async function( ) {
  let index = 0;
  for (let tab of tabs ) {
    await browser.tabs.create({
      url: tab.url,
      index: index
    });
    index++;
  }
}

Comment 5

[Bob Silverberg [:bsilverberg]]

I don't understand what your question is. Your original report was about a problem with creating bookmarks, but your new example is only about creating tabs. What exactly are you trying to achieve, and what is the exact problem you are trying to overcome?

Comment 6

[Morikko]

Sorry for the confusion:

* My initial problem was about the bookmarks. And you solved it.

* I explained you that I did the mistake because I believed index was "reserved" at function call and I didn't have to wait the full creation for adding another bookmark after.

* I believed in this behavior because it seems to be the case for tabs creation with tabs.create. 

Now, I would like to ask another question about tabs.create: Can I always be sure index will be respect without waiting complete tab creation ?
 * I found nothing in documentation
 * Neither in Bugzilla

Do I need to create another thread in Bugzilla, or there is another way for asking questions like this (as it is not a bug) ?

Again, sorry for mixing everything.

Comment 7

[Bob Silverberg [:bsilverberg]]

(In reply to Morikko from comment #6)
> Sorry for the confusion:
> 
> * My initial problem was about the bookmarks. And you solved it.
> 
> * I explained you that I did the mistake because I believed index was
> "reserved" at function call and I didn't have to wait the full creation for
> adding another bookmark after.
> 
> * I believed in this behavior because it seems to be the case for tabs
> creation with tabs.create. 
> 
> Now, I would like to ask another question about tabs.create: Can I always be
> sure index will be respect without waiting complete tab creation ?
>  * I found nothing in documentation
>  * Neither in Bugzilla
> 
> Do I need to create another thread in Bugzilla, or there is another way for
> asking questions like this (as it is not a bug) ?
> 
> Again, sorry for mixing everything.

No problem. In the future the best way to ask questions is probably to send an email to [e-mail].

Regarding this question, looking at the code for tabs.create, it looks like there could be a case where calling it in succession will not guarantee that the tabs are created in that order (if the window isn't ready yet), but that that is highly unlikely to occur. I think you can count on the tabs being created in the correct order, and therefore having the expected indexes, without waiting for each promise to resolve.

aswan and/or kmag, can you confirm this?

Comment 8

[Andrew Swan [:aswan]]

(In reply to Bob Silverberg [:bsilverberg] from comment #7)
> Regarding this question, looking at the code for tabs.create, it looks like
> there could be a case where calling it in succession will not guarantee that
> the tabs are created in that order (if the window isn't ready yet), but that
> that is highly unlikely to occur. I think you can count on the tabs being
> created in the correct order, and therefore having the expected indexes,
> without waiting for each promise to resolve.

I don't know off the top of my head what the current implementation happens to do but you shouldn't rely on any particular undocumented behavior.  If you want to be certain that tabs will be created in a particular order, the reliable way to do that is to fully create the first before starting the second, and son on.