Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Bookmarks cannot be moved to Other Bookmarks folder #10157

Closed
luixxiul opened this issue Jul 26, 2017 · 14 comments
Closed

Bookmarks cannot be moved to Other Bookmarks folder #10157

luixxiul opened this issue Jul 26, 2017 · 14 comments
Assignees
@NejcZdovc
Labels
bug feature/about-pages feature/bookmarks QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Milestone
0.20.x (Release C...

Comments

@luixxiul
Copy link
Contributor

luixxiul commented Jul 26, 2017 •
edited
Loading

Test Plan

See below

Describe the issue you encountered: Bookmarks cannot be moved to Other Bookmarks folder.

bug

In the gif the bookmark is moved to the last row inside Bookmarks Toolbar folder.

  • Platform (Win7, 8, 10? macOS? Linux distro?): Debian

  • Brave Version (revision SHA): 0.18.12

  • Steps to reproduce:

    1. Bookmark a page
    2. Open about:bookmarks
    3. Drag and drop the bookmark to Other Bookmarks

  • Actual result: the bookmark is not moved to the folder

  • Expected result: the bookmark should be moved to the folder

  • Extra QA steps:
    1.
    2.
    3.

  • Any related issues: Bookmarks in Other Bookmarks folder is synced to the bookmark toolbar #8024
@luixxiul luixxiul added this to the 0.20.x (Nightly Channel) milestone Jul 26, 2017
@NejcZdovc
Copy link
Contributor

Yes this is because we only allow 0+ (not -1) for parent folder id. I notice this as well while I was working on my PR as well

@luixxiul
Copy link
Contributor Author

fyi it has been possible to do so when I opened #8024

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Jul 26, 2017 •
edited
Loading

yeah I think that this is a regression

@eljuno
Copy link
Contributor

eljuno commented Jul 26, 2017 •
edited
Loading

+1 from community

Like the record, moving bookmarks from Bookmark Toolbar folder and it's subfolder to Other Bookmarks folder will move that bookmarks to Bookmark Toolbars folder not Other Bookmarks.

@LaurenWags
Copy link
Member

LaurenWags commented Jul 27, 2017 •
edited
Loading

Similarly with moving a folder to 'Other Bookmarks'.

ubuntuotherfolder

luzmcosta added a commit to luzmcosta/browser-laptop that referenced this issue Aug 6, 2017
@luzmcosta
Fixes dragging into/from Other Bookmarks
eb78a10 
Fixes brave#10157

Bookmarks could not be moved to the Other
Bookmarks folder, where archived bookmarks
reside. The reason surrounded two issues, as
follows:

The first issue was in the renderer process,
which did not correctly set the parentFolderId
on  the keys of bookmarks in child folders. e.g.
Bookmarks Toolbar > Test Folder > Bookmark.

The second issue was in the main process, which
was not updating the `parentFolderId` property
of the old bookmark before setting it anew.

I’ve set two placeholder tests, skipped for now,
as I don’t want to hold up value for the user
while I find my way around what tests already
exist.
@luzmcosta luzmcosta mentioned this issue Aug 6, 2017
Closed
8 tasks
@luixxiul luixxiul modified the milestones: 0.21.x (Nightly Channel), 0.20.x (Developer Channel) Aug 6, 2017
@luzmcosta luzmcosta mentioned this issue Aug 7, 2017
Closed
@KutX9JtmXrZP
Copy link

+1
FWIW I am having this problem as well. The "Other Bookmarks" folder might as well not exist.

Brave: 0.18.23
rev: 36ae2ec
Muon: 4.3.10
libchromiumcontent: 60.0.3112.90
V8: 6.0.286.52
Node.js: 7.9.0
Update Channel: dev
OS Platform: Microsoft Windows
OS Release: 6.1.7601
OS Architecture: x64

@NejcZdovc NejcZdovc self-assigned this Aug 17, 2017
@NejcZdovc NejcZdovc mentioned this issue Aug 18, 2017
Closed
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Aug 21, 2017
@NejcZdovc
Fixes Other bookmarks not being usable
440661c 
Resolves brave#10157
Resovles brave#4202

Auditors:

Test Plan:
@NejcZdovc NejcZdovc mentioned this issue Aug 21, 2017
Merged
8 tasks
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Aug 21, 2017
@NejcZdovc
Fixes Other bookmarks not being usable
e021002 
Resolves brave#10157
Resolves brave#4202

Auditors:

Test Plan:
@ghost ghost added the sprint/1 label Sep 13, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Sep 18, 2017
@NejcZdovc
Fixes Other bookmarks not being usable
d6de9fc 
Resolves brave#10157
Resolves brave#4202

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Sep 26, 2017
@NejcZdovc
Fixes Other bookmarks not being usable
33b6788 
Resolves brave#10157
Resolves brave#4202

Auditors:

Test Plan:
cezaraugusto added a commit that referenced this issue Sep 28, 2017
@cezaraugusto
Merge pull request #10610 from NejcZdovc/hotfix/#10157-otherBookmarks
8d76701 
Fixes Other bookmarks not being usable
@Cavadus
Copy link

Cavadus commented Oct 8, 2017

+1 on this issue. Cannot move a single bookmark into "Other Bookmarks."

Brave: 0.18.36
rev: 7ab85e9
Muon: 4.3.22
libchromiumcontent: 61.0.3163.79
V8: 6.1.534.32
Node.js: 7.9.0
Update Channel: dev
OS Platform: Microsoft Windows
OS Release: 10.0.15063
OS Architecture: x64

@NejcZdovc
Copy link
Contributor

@Cavadus this will be fixed in version 0.21.x

@bbondy bbondy modified the milestones: 0.21.x (Developer Channel), 0.20.x (Beta Channel) Oct 25, 2017
syuan100 pushed a commit to syuan100/browser-laptop that referenced this issue Nov 9, 2017
@NejcZdovc @syuan100
Fixes Other bookmarks not being usable
393f1d8 
Resolves brave#10157
Resolves brave#4202

Auditors:

Test Plan:
@jamesray1
Copy link

jamesray1 commented Nov 10, 2017 •
edited
Loading

+1. Couldn't today. When I created a folder in Other Bookmarks, I could move bookmarks to that folder.

Brave: 0.19.80
rev: 7d07299
Muon: 4.5.13
libchromiumcontent: 62.0.3202.75
V8: 6.2.414.36
Node.js: 7.9.0
Update Channel: Release
OS Platform: Linux
OS Release: 4.13.0-17-generic
OS Architecture: x64

@jamesray1 jamesray1 mentioned this issue Nov 10, 2017
Closed
@NejcZdovc
Copy link
Contributor

@jamesray1 this was fixed in 0.20 version, so in the next major release

This was referenced Dec 20, 2017
Closed
Closed
@LaurenWags
Copy link
Member

@NejcZdovc should this work when moving a folder to 'Other Bookmarks' (see #10157 (comment))? Because right now the folder you move disappears from Bookmark Manager but stays on the Toolbar:
10157

cc @kjozwiak @srirambv

@NejcZdovc
Copy link
Contributor

yes it should work

@NejcZdovc
Copy link
Contributor

can you please open new issue for this one, because steps are not the same as a main issue

@NejcZdovc NejcZdovc mentioned this issue Dec 22, 2017
Closed
@NejcZdovc
Copy link
Contributor

issue created #12378

@NejcZdovc NejcZdovc mentioned this issue Jan 11, 2018
Closed
@kjozwiak kjozwiak mentioned this issue Jan 31, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@NejcZdovc NejcZdovc

Labels
bug feature/about-pages feature/bookmarks QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Projects
None yet
Milestone
0.20.x (Release Channel)
Development

No branches or pull requests
9 participants
@bbondy @luixxiul @NejcZdovc @Cavadus @KutX9JtmXrZP @jamesray1 @srirambv @eljuno @LaurenWags