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

Fixes context menu not working on tabsbar #9525

Merged
merged 1 commit into from
Jun 19, 2017

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Jun 16, 2017

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.

Resolves #9524

Auditors: @bsclifton

Test Plan:

  • right click on tabs toolbar (on empty space)
  • context menu should open

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

Resolves brave#9524

Auditors: @bsclifton

Test Plan:
- right click on tabs toolbar (on empty space)
- context menu should open
@NejcZdovc NejcZdovc added this to the 0.17.x (Beta Channel) milestone Jun 16, 2017
@NejcZdovc NejcZdovc self-assigned this Jun 16, 2017
@NejcZdovc NejcZdovc requested a review from bsclifton June 16, 2017 17:15
@NejcZdovc NejcZdovc added regression and removed bug labels Jun 16, 2017
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works good on macOS, but doesn't work on Windows. Because of the style attributes required for dragging, the context menu doesn't auto-show

You may want to take a look at #8320, which solves this problem (and also adds a more usable menu). It ended up being reverted because of hitbox issues which may be solved now

@NejcZdovc
Copy link
Contributor Author

This issue is not solving any specific problems as you fond on windows. This PR only fixes a regression where we are passing wrong params into the function. We can track issue for this problem in #8320.

@bsclifton
Copy link
Member

@NejcZdovc gotcha- works for me 😄

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

@bsclifton bsclifton merged commit fab48e2 into brave:master Jun 19, 2017
bsclifton added a commit that referenced this pull request Jun 19, 2017
Fixes context menu not working on tabsbar
bsclifton added a commit that referenced this pull request Jun 19, 2017
Fixes context menu not working on tabsbar
@srirambv
Copy link
Collaborator

srirambv commented Jun 21, 2017

9524

I see the bookmarks toolbar context menu show up in a specific place (right on the edge of the tabs bar and tabset) when I right click on the empty space. Other places it bring up the browser context menu. @NejcZdovc @bsclifton Is this expected ?

@NejcZdovc
Copy link
Contributor Author

NejcZdovc commented Jun 21, 2017

@srirambv context menu in bookmarks toolbar is not working for windows. More info here #4244 (comment). This PR only fixes macOS bug.

@bsclifton
Copy link
Member

bsclifton commented Jun 21, 2017

@srirambv what @NejcZdovc said 😄

#4244 will need to be solved in order to have draggable areas that don't return the Windows system menu (ex: same menu that shows when you right click on a titlebar)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants