Closed
Bug 1416573
Opened 7 years ago
Closed 7 years ago
bookmarks.create, index is not taking in account
Categories
(WebExtensions :: General, defect)
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: rico.masseran, Assigned: bsilverberg)
Details
(Whiteboard: [bookmarks])
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).
Updated•7 years ago
|
Component: Untriaged → WebExtensions: General
Product: Firefox → Toolkit
Assignee | ||
Comment 1•7 years ago
|
||
I will investigate.
Assignee: nobody → bob.silverberg
Whiteboard: investigate[bookmarks]
Assignee | ||
Comment 2•7 years ago
|
||
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); } }
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
Whiteboard: investigate[bookmarks] → [bookmarks]
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++; } }
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++; } }
Assignee | ||
Comment 5•7 years ago
|
||
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?
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.
Assignee | ||
Comment 7•7 years ago
|
||
(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 dev-addons@mozilla.org. 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?
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(aswan)
Comment 8•7 years ago
|
||
(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.
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(aswan)
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•