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

Open bookmarks in new foreground tabs #5831

Conversation

GreenRecycleBin
Copy link
Contributor

@GreenRecycleBin GreenRecycleBin commented Nov 24, 2016

This commit should fix #3207.

There doesn't seem to be any middle-clicking bookmark item(s) test under https://github.com/brave/browser-laptop/blob/ca626503925c6f706a65896b1b679692374d6697/test/components/bookmarksToolbarTest.js.

In order to add some tests according to the test plans I've outlined in the commits, I'd need to know how to:

  • Simulate middle clicking on bookmark item(s)
  • Whether to do a URL match of a new foreground tab to that of the original bookmark item or check that such an event is fired

I can probably study other tests and figure out how to do these. But it'd be helpful if someone could give me a few pointers. Thank you.

@GreenRecycleBin
Copy link
Contributor Author

A related question is whether we should implement Shift + middle clicking a bookmark folder to open in new foreground tabs regardless of the value of SWITCH_TO_NEW_TABS setting? This parallels the current behavior of Shift + middle clicking a bookmark item to always open it in a new foreground tab.

@GreenRecycleBin
Copy link
Contributor Author

@bbondy Could you help review this?

If SWITCH_TO_NEW_TABS is truthy (or if Shift key is pressed: this is the
current behavior)

Auditors: @bsclifton

Test Plan:
  - Simulate middle clicking a bookmark item
  - Check that a new foreground tab is opened with the same URL as that of the
  bookmark (or just check that if such an event is fired if a URL check is not
  feasible)

Fix brave#3207.
@bsclifton bsclifton force-pushed the feature/open-bookmarks-in-new-foreground-tabs branch from 85e51a9 to 87d9bf2 Compare November 26, 2016 08:00
@bsclifton bsclifton added this to the 0.13.0 milestone Nov 26, 2016
@bsclifton bsclifton self-assigned this Nov 26, 2016
@bsclifton
Copy link
Member

Awesome, thanks for the fix, @GreenRecycleBin!

I manually tested and verified this works as I'd expect. If you wanted to add a test (which I'd highly encourage you to do and submit another PR for), you identified a good place to add the test 😄

Here's a link to our test doc, which includes a link to the webdriver documentation. To simulate a middle click, you'd want to use the middleClick action:
http://webdriver.io/api/action/middleClick.html

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.

Middle click on a bookmark link opens tab in the background
2 participants