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

Editing a bookmark (or folder) locks up the browser #9674

Closed
bsclifton opened this issue Jun 23, 2017 · 4 comments · Fixed by #9710
Closed

Editing a bookmark (or folder) locks up the browser #9674

bsclifton opened this issue Jun 23, 2017 · 4 comments · Fixed by #9710
Assignees
@NejcZdovc
Labels
feature/bookmarks perf QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Milestone
0.19.x (Beta Chan...

Comments

@bsclifton
Copy link
Member

bsclifton commented Jun 23, 2017 •
edited by luixxiul
Loading

Test plan

#9710 (comment)

  • Did you search for similar issues before submitting this one?
    Yes

  • Describe the issue you encountered:
    Bringing up the modal used to add/edit bookmarks/folders is horribly slow

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

  • Brave Version (revision SHA):
    0.17.6

  • Steps to reproduce:

    1. Have a decent number of bookmarks (maybe 500 - 1,000) and a good amount of history
    2. Right click on bookmarks toolbar, pick Add folder
    3. Browser is frozen
    4. Wait 4 seconds or more for the modal to show up
    5. Try typing into the textbox (giving a name to the folder)
    6. Notice that typing is extremely lagged. It's unusable

  • Actual result:
    The browser freezes up and visibly shows the lag (often turning mouse into beach ball icon)

  • Expected result:
    The UI should be responsive and snappy

  • Will the steps above reproduce in a fresh profile? If not what other info can be added?

  • Is this an issue in the currently released version?

  • Can this issue be consistently reproduced?

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:

  • Any related issues:
@bsclifton bsclifton added the perf label Jun 23, 2017
@bsclifton bsclifton mentioned this issue Jun 23, 2017
Closed
35 tasks
@bsclifton
Copy link
Member Author

Assigning to @NejcZdovc; this may be improved by some of the upcoming redux component work / optimizations

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Jun 26, 2017 •
edited
Loading

Ok found one real bottle necks and one improvment.

First: https://github.com/brave/browser-laptop/blob/master/app/renderer/components/bookmarks/addEditBookmarkHanger.js#L181

If I disabled this line I get computed time for merge props:
image

If I enabled the first one:
image

(this one can be improved, but it's not a bottle neck) Second: https://github.com/brave/browser-laptop/blob/master/app/renderer/components/bookmarks/addEditBookmarkHanger.js#L187

For the second one instead of using find we could use some, because we only need to know if at least one bookmark or folder is in site list.

Test was done with bookmark export provided by @bsclifton (it has lizard people in and some other bookmarks cca 6k)

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jun 26, 2017
@NejcZdovc
Imporves perf of bookmark hanger
80ccc20 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
@NejcZdovc NejcZdovc mentioned this issue Jun 26, 2017
Merged
8 tasks
@NejcZdovc NejcZdovc added this to the 0.18.x (Developer Channel) milestone Jun 27, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 4, 2017
@NejcZdovc
Imporves perf of bookmark hanger
a791884 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 5, 2017
@NejcZdovc
Imporves perf of bookmark hanger
b56026b 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 5, 2017
@NejcZdovc
Imporves perf of bookmark hanger
122ada5 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 5, 2017
@NejcZdovc
Imporves perf of bookmark hanger
765c713 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 7, 2017
@NejcZdovc
Imporves perf of bookmark hanger
3fabe4a 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 7, 2017
@NejcZdovc
Imporves perf of bookmark hanger
fc32cc7 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 7, 2017
@NejcZdovc
Imporves perf of bookmark hanger
0a67879 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
@NejcZdovc NejcZdovc modified the milestones: 0.19.x (Developer Channel), 0.18.x (Beta Channel) Jul 9, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 11, 2017
@NejcZdovc
Imporves perf of bookmark hanger
c0372f1 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 11, 2017
@NejcZdovc
Imporves perf of bookmark hanger
21e2b13 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 11, 2017
@NejcZdovc
Imporves perf of bookmark hanger
8a38142 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 11, 2017
@NejcZdovc
Imporves perf of bookmark hanger
4538db1 
Resolves brave#9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
bsclifton added a commit that referenced this issue Jul 11, 2017
@bsclifton
Merge pull request #9710 from NejcZdovc/hotfix/#9674-bookmark
c8e92a1 
Improves perf of bookmark hanger
NejcZdovc added a commit that referenced this issue Jul 11, 2017
@NejcZdovc
Imporves perf of bookmark hanger
ca4ceca 
Resolves #9674

Auditors: @bsclifton

Test Plan:
- have a lot of bookmarks (1k +)
- try to add a bookmark with clicking on the star
@bigkahuna1uk
Copy link

This is still happening for the latest Brave release. Browser is completely locked up and has to be killed.

Name Version
Brave 0.18.14
rev ad92d02
Muon 4.3.6
libchromiumcontent 60.0.3112.78
V8 6.0.286.44
Node.js 7.9.0
Update Channel dev
OS Platform macOS
OS Release 16.7.0
OS Architecture x64

@NejcZdovc
Copy link
Contributor

@bigkahuna1uk this will be fixed in 0.19.x version. Coming really soon

@cndouglas cndouglas mentioned this issue Sep 7, 2017
Closed
This was referenced Sep 20, 2017
Closed
Closed
Closed
Closed
This was referenced Sep 20, 2017
Closed
Closed
This was referenced Sep 21, 2017
Closed
Closed
This was referenced Sep 25, 2017
Closed
Closed
@kjozwiak kjozwiak mentioned this issue Sep 27, 2017
Closed
@srirambv srirambv mentioned this issue Sep 28, 2017
Closed
@srirambv srirambv mentioned this issue Oct 9, 2017
Closed
@kjozwiak kjozwiak mentioned this issue Oct 12, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@NejcZdovc NejcZdovc

Labels
feature/bookmarks perf QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Projects
None yet
Milestone
0.19.x (Beta Channel)
Development

Successfully merging a pull request may close this issue.

Improves perf of bookmark hanger NejcZdovc/browser-laptop
6 participants
@bigkahuna1uk @luixxiul @bsclifton @NejcZdovc @srirambv @LaurenWags