-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Other bookmarks is missing, breaking bookmarks API #7639
Comments
Unfortunately with this new patch the Other Bookmarks folder is no longer anchored to it's own area on the right hand side of the bookmarks bar. This is an issue for me personally and not expected behaviour from a chromium based browser. (and honestly although seemingly minor this change, if intended, will most likely force me to stop using Brave and it's really disruptive to my workflow) I'm hoping this can be fixed somehow. |
per @rob4226 via #5158 (comment)
|
Going to re-open this; @rebron and @darkdh are looking into this, after comment by @nero120 via #5158 (comment)
|
I wasn't personally a big fan of the change to remove "Other Bookmarks", but (unfortunately ;) I don't always get my way. In any case I can tell you with a high degree of certainty that this change is here to stay. We have had extensive (and heated) discussions about this and it's not likely to change. What could change here is how things are presented in the bookmarks api so that Brave looks like it has the same bookmark structure as Chrome. Basically a kind of "shadow" node that is actually a reference to the bookmarks bar, but appears to be "other bookmarks" in the api. However, breakage in the known extensions could easily be fixed without any changes to Brave and at least one of them relies on undocumented node ids which are not part of the API and hence not even guaranteed to be compatible across chromium versions |
( Moving discussion here from #5158 )
Agree this is disappointing. For what it's worth, Tabli doesn't depend on a fixed node ID. It does depend on The only mention I've found about the contents of the root folder in the API docs is this (on the After this change in Brave, are there any guarantees about what will be in the root folder in Brave? @bridiver suggested either leaving the parent id blank or using I'd like to do the right thing for Brave users. My hunch is that my extension creating a "Tabli Saved Windows" folder on the Bookmarks Bar in Brave isn't going to be an ideal user experience. "Other Bookmarks" is really a pretty reasonable place for this, but it's not entirely obvious to me if there is an approved replacement for that in Brave. |
FYI, you can't really query by name (title) since they are different for each language! |
If Brave provided a root node with a replacement folder for "Other Bookmarks" at position 1 in the children list, I'm pretty sure this would instantly fix the Tabli extension. |
This is really disappointing and very user hostile. Moved my entire family over to Brave during the holidays for improved security and woke up to txts this morning asking why their bookmarks have moved around. With Chrome controlling ~70% of the browser market, the vast majority of people switching to Brave expect the "Other Bookmarks" folder to be on the right, not mixed in with their normal bookmarks. |
Even after solving #5158 the sync issues still remain though. Trying to sync my Mac with android ended up with duplicate folders or missing bookmarks. |
You can always create a folder inside the bookmarks bar, but I seriously doubt we'll change the node structure because we wanted to standardize it across platforms (unlike Chrome which has a separate "mobile bookmarks" folder)
… On Jan 8, 2020, at 3:45 PM, Antony Courtney ***@***.***> wrote:
( Moving discussion here from #5158 )
@nero120 I was surprised they didn't have constants for the bookmarks bar and other bookmarks node. I expected to find them in the api docs and use it as a potential fix inside the api
Agree this is disappointing.
For what it's worth, Tabli doesn't depend on a fixed node ID. It does depend on tree.children[1] being the "Other Bookmarks" folder, where tree is the root returned from chrome.bookmarks.getTree.
Not great, although I also think that looking for "Other Bookmarks" by name off the root also would have broken with this change.
The only mention I've found about the contents of the root folder in the API docs is this (on the chrome.bookmarks API page):
After this change in Brave, are there any guarantees about what will be in the root folder in Brave?
Is "Other Bookmarks" replaced by a folder named "Bookmarks", a folder that exists at the same level as "Bookmarks", a sub-folder of "Bookmarks", or something else? It's really not clear from all the screenshots above.
@bridiver suggested either leaving the parent id blank or using children.length - 1 as the index when creating the folder used by my extension.
I'd like to do the right thing for Brave users. My hunch is that my extension creating a "Tabli Saved Windows" folder on the Bookmarks Bar in Brave isn't going to be an ideal user experience. "Other Bookmarks" is really a pretty reasonable place for this, but it's not entirely obvious to me if there is an approved replacement for that in Brave.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
There won't be a replacement per se, but we may decide to fake it to avoid other potential breakage. In that case I think it's likely that "Other Bookmarks" would just be an alias for the bookmarks bar
… On Jan 8, 2020, at 4:01 PM, Antony Courtney ***@***.***> wrote:
I wasn't personally a big fan of the change to remove "Other Bookmarks", but (unfortunately ;) I don't always get my way. In any case I can tell you with a high degree of certainty that this change is here to stay. We have had extensive (and heated) discussions about this and it's not likely to change. What could change here is how things are presented in the bookmarks api so that Brave looks like it has the same bookmark structure as Chrome. Basically a kind of "shadow" node that is actually a reference to the bookmarks bar, but appears to be "other bookmarks" in the api. However, breakage in the known extensions could easily be fixed without any changes to Brave and at least one of them relies on undocumented node ids which are not part of the API and hence not even guaranteed to be compatible across chromium versions
If Brave provided a root node with a replacement folder for "Other Bookmarks" at position 1 in the children list, I'm pretty sure this would instantly fix the Tabli extension.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
This is something we have discussed and it seems silly to me that we don't match up bookmarks by name/url/folder. I can't see any reason why anyone would intentionally create a duplicate of a bookmark in the same folder cc @davidtemkin, however that is a separate issue that should have its own ticket. Also there are other outstanding issues with the sync implementation that are being fixed for missing bookmarks, etc... |
@antonycourtney @edsimpsons83 This was a mistake, we will fix as best we can. Sorry about it and please stay tuned. |
It would be nice to get my xBrowserSync extension working again. Only started using Brave since November last year after considering many alternative browsers. |
Yes, due to lack of 1st-party implementations for every platform (talking about things like Arch Linux here) I have a need for other browsers sometimes, so xBrowserSync functionality is a must. It sounds like based off of what @BrendanEich is saying that you guys are planning on undoing the removal of the dedicated "Other Bookmarks" folder, but if you go the route of making "Other Bookmarks" an alias, please do not make it for the Bookmarks Bar, but instead a folder called "Other Bookmarks" that can be put wherever on the Bookmarks Bar. Otherwise we'll have a mess on our hands with any extensions that sync Bookmarks. |
For me it happened that after brave auto-updated, all my 2000+ bookmarks disappeared, because https://www.everhelper.me/synchronizer.php couldn't deal with the changes. @nsy16 and others, I recommend to install Brave version 1.1.23 temporarily to have the old bookmarks structure back and sync it, works for me. |
Fix has landed and should now be in Nightly (1.7, which is slated for Release channel on April 28). Next step would be planning the back-port of this (ex: Dev 1.6/April 7, Beta 1.5/Mar 17, Release 1.4/Feb 25) @kjozwiak @rebron @davidtemkin would this be something we want for 1.4? (upcoming release channel). Or would we want to shoot for 1.5 or 1.6? (give the fix a little more time to bake) |
To summarize: There was some difficulty with bookmarks syncing, and it was determined that the complicated and ill-defined bookmarks structure inherited from Chrome was preventing a clean solution. Now, after the migration to a simpler structure has been in the field, it is decided to revert to the original structure. Our brave developers shall, without noticeable disruption, reverse-migrate users, some of whom may have several synced devices, some migrated and some not, using various versions of Brave, back to the original structure. And then, I presume, solve the original syncing difficulty. This will be a very impressive accomplishment. |
@jerrykrinock out of interest, what exactly was so difficult about syncing the original chrome bookmarks structure? |
@nero120 First of all, I'm not with Brave. I am just an bookmarks-related extension deveIoper who subscribed to this issue because I need to support Brave's past and future changes. To answer your question, I'm just recalling what I read, maybe it was in the Brave forums or maybe it was in the original issue wherein they decided to remove Other Bookmarks. And I do recall that, in the original 2009 chrome.bookmarks API, the Other Bookmarks was not properly defined by Google, so I had to do some reverse engineering. I suspect it has something to do with the fact that synchronizing a tree is complicated enough if there is only one "hard" root item. |
Another update, another version and another disappointment. |
I am right in thinking this fix isn't in Version 1.4.95? |
@stmuk this is currently only in Nightly (1.7) Per our release schedule, 1.7 is scheduled to be publicly released on April 28 @rebron @kjozwiak should this fix be uplifted to 1.6? (scheduled for April 7). It's definitely too late to pull into 1.5 (scheduled for March 17) |
Please correct the problem as soon as possible. |
FWIW, @bsclifton I would say to try to uplift into 1.6. |
Actually- uplift may not be needed... current plan is to expedite 1.7 to April 7 😄 (skipping the 1.6 version) |
I see that 1.5 is now in production and 1.7 is now in beta. So is this true that 1.6 is being skipped? And that if anyone does have 1.6 (from a nightly or whatever), that 1.6 does not have Other Bookmarks? |
I've been visiting the "About Brave" today to update brave and finally resolve this issue that is affecting us since January, but until now, no update. Do you know when the update will be released? cc @bsclifton |
Verified passed with
Verified test plan from brave/brave-core#4404 Migrate previous migration - Pass
Virtual "Other Bookmarks" Mapping
Other Scenarios I executed:
Verification is in progress
Migrate previous migration
Virtual "Other Bookmarks" mapping
Device A windows 1.7.x, Device B windows 1.8.x, Device C Android
Device A windows 1.7.x, Device B windows 1.8.x, Device C Android
Device A windows 1.7.x, Device B windows 1.8.x, Device C Android
Device A windows 1.7.x, Device B windows 1.8.x, Device C Android
Device A Desktop 1.7.x, Device B Desktop 1.8.x, Device C iPhone 8
Verification passed on
Migrate previous migration
Virtual "Other Bookmarks" mapping
Device A Linux 1.7.x, Device B Linux 1.7.x, Device C Android
Device A Linux 1.7.x, Device B Linux 1.7.x, Device C Android
Device A Linux 1.7.x, Device B Linux 1.7.x, Device C Android
Device A Linux 1.7.x, Device B Linux 1.7.x, Device C Android
|
Hi @gabrielmulle. We have a candidate build out now that we're testing, 1.7.86 and looking to make it available this Thursdaay. |
@jerrykrinock Sorry for the super late response but yes 1.6 did not have other bookmarks. Other bookmarks was added back in 1.7.6. |
Thank you, @rebron. I found the release calendar a couple weeks ago. On April 3 I published an update to my apps which will supposedly handle bookmark imports and exports with all versions of Brave, with or without the hard Other Bookmarks. I also posted a support article explaining the situation and advising users to ignore the warnings and circuit breaker tripping which will presumably occur when large numbers of bookmarks are moved back into the resurrected hard Other Bookmarks. |
Dear Brave,
isn't it frustrating if after the update to version 1.2.41 instead of "Other bookmarks"only ">>" sees?!
The bookmark changes are unnecessary, bring no benefits and only annoy the users.
The structure and layout of the bookmarks has proven itself for many years. There was no reason to change this. Brave is now no longer compatible with other Chromium based browser.
Please undo changes to bookmarks.
Thank you
The text was updated successfully, but these errors were encountered: