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

Bookmark toolbar context menu not working on 0.20.x #11728

Closed
srirambv opened this issue Oct 31, 2017 · 12 comments
Closed

Bookmark toolbar context menu not working on 0.20.x #11728

srirambv opened this issue Oct 31, 2017 · 12 comments
Assignees
@petemill @bsclifton
Labels
0.20.x issue first seen in 0.20.x bug feature/bookmarks feature/context-menu OS/Windows QA/checked-Linux QA/checked-Win64 QA/test-plan-specified release/blocking release-notes/exclude
Milestone
0.20.x (Release C...

Comments

@srirambv
Copy link
Collaborator

srirambv commented Oct 31, 2017 •
edited by bsclifton
Loading

Test plan

See #12676

Description

Bookmark toolbar context menu not working on 0.20.x

Steps to Reproduce

  1. Build from a42cac1
  2. Add a bookmark to enable bookmark toolbar
  3. Right click anywhere on the tool bar, does't show toolbar context menu instead shows window context menu

Actual result:
bmkcontext

Expected result:
Should show bookmark toolbar context menu when right clicked on toolbar

Reproduces how often: [What percentage of the time does it reproduce?]
100%

Brave Version

about:brave info:

Brave 0.20.1
V8 6.2.414.36
rev a42cac1
Muon 4.5.11

Reproducible on current live release:
No

Additional Information

Found while verifying #11672
@srirambv srirambv added this to the 0.20.x (Beta Channel) milestone Oct 31, 2017
@NejcZdovc
Copy link
Contributor

@srirambv if I am not mistaken this is on windows only right?

@srirambv
Copy link
Collaborator Author

Correct. Found on Windows, @LaurenWags @kjozwiak could verify for macOS

@kjozwiak
Copy link
Member

kjozwiak commented Nov 1, 2017

Correct. Found on Windows, @LaurenWags @kjozwiak could verify for macOS

I couldn't reproduce this on either macOS nor Ubuntu... Went through the following checks:

  • macOS 10.12.6 x64 on the 0.20.x branch using bafa261 - Couldn't Reproduce
  • Ubuntu 17.04 x64 on the 0.20.x branch using bafa261 - Couldn't Reproduce
  • Win 10 Pro x64 on the 0.20.x branch using bafa261 - Reproduced

@luixxiul luixxiul added the 0.20.x issue first seen in 0.20.x label Nov 1, 2017
@darkdh darkdh self-assigned this Nov 3, 2017
@bsclifton
Copy link
Member

This has been reported many times already- it's a known issue, root cause captured with #4244

@bsclifton bsclifton added duplicate Issue has already been reported and removed 0.20.x issue first seen in 0.20.x regression labels Nov 3, 2017
@bsclifton bsclifton removed this from the 0.20.x (Beta Channel) milestone Nov 3, 2017
@srirambv
Copy link
Collaborator Author

@bsclifton this is a blocker on 0.20.8. Always shows system context menu on bookmark toolbar. Could #4244 be prioritized to 0.20.x so that it fixes the issue?

@bsclifton
Copy link
Member

@srirambv this has been existing behavior for as long as we've switched to the version without titlebar (possibly earlier). Are you positive you're not seeing it with 0.19.x?

@srirambv
Copy link
Collaborator Author

Doesn't happen on the release build
11728

I remember it got fixed by some PR a long time back but haven't seen this happen on most recent releases as adding bookmark from toolbar is part of general regression test run so it would have been captured if it happened on 0.19.x

@bsclifton bsclifton reopened this Dec 21, 2017
@bsclifton
Copy link
Member

@srirambv thanks for confirming- this new release should work as good as the one before. Maybe a style update affected this. If the area is marked as draggable, it will cause that issue

@luixxiul can you help me look at the area in question? perhaps there was an unintentional regression

@srirambv
Copy link
Collaborator Author

Another interesting thing is if the toolbar doesn't have any items then right at the edge of the tabs bar the context menu shows up but once you add any item to the toolbar the context menu disappears
11728-1

@srirambv srirambv removed the duplicate Issue has already been reported label Dec 22, 2017
This was referenced Dec 22, 2017
Closed
Closed
@luixxiul luixxiul added 0.20.x issue first seen in 0.20.x and removed 0.20.x issue first seen in 0.20.x labels Dec 23, 2017
@srirambv
Copy link
Collaborator Author

srirambv commented Jan 9, 2018

Adding it back to 0.20.x as its blocking couple of other issue to be verified

@srirambv srirambv added this to the 0.20.x (Beta Channel) milestone Jan 9, 2018
@srirambv srirambv added the 0.20.x issue first seen in 0.20.x label Jan 9, 2018
@NejcZdovc
Copy link
Contributor

yeah not just bookmarks but tabs as well, more info here #12590.

@NejcZdovc NejcZdovc mentioned this issue Jan 10, 2018
Closed
@bsclifton
Copy link
Member

Likely same fix as #12590

petemill added a commit that referenced this issue Jan 17, 2018
@petemill
Restore disabled dragging for bookmarks bar on windows.
cbcedd2 
Previously it was enough to not add 'drag' to the bookmarks bar on Windows, but not that it is added at a higher level in the DOM (so that dragging works in the margin between the bars) via #10512, we have to explicitly set it to 'no-drag' on each child.

Fix #11728
petemill added a commit that referenced this issue Jan 17, 2018
@petemill
Restore disabled dragging and context menu for Bookmarks Bar on Windows
79b93e1 
Previously it was enough to not add 'drag' to the bookmarks bar on Windows, but now that it is added at a higher level in the DOM (so that dragging works in the margin between the bars) via #10512, we have to explicitly set it to 'no-drag' on each child.

Fix #11728
@petemill petemill mentioned this issue Jan 17, 2018
Merged
10 tasks
@LaurenWags LaurenWags mentioned this issue Jan 22, 2018
Closed
19 tasks
@srirambv srirambv mentioned this issue Jan 24, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@petemill petemill

@bsclifton bsclifton

Labels
0.20.x issue first seen in 0.20.x bug feature/bookmarks feature/context-menu OS/Windows QA/checked-Linux QA/checked-Win64 QA/test-plan-specified release/blocking release-notes/exclude
Projects
None yet
Milestone
0.20.x (Release Channel)
Development

No branches or pull requests
8 participants
@petemill @alexwykoff @kjozwiak @luixxiul @bsclifton @NejcZdovc @darkdh @srirambv