Skip to content
This repository has been archived by the owner. It is now read-only.

Adds a session selector to bookmark dialog #14185

Closed
wants to merge 437 commits into from

Conversation

@ishanrp
Copy link

ishanrp commented May 20, 2018

Fixes #7417

Adds a session selector to the add/edit bookmark dialog. When opened in a new tab the bookmark will be opened in the selected session.

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.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

Adding using context menu

  1. Add new bookmark using the context menu.
  2. The current session should already be selected or select another session.
    Result: Opening the bookmark in new tab should open in the selected session.

add-bookmark

Using star to instant add

  1. Click on the bookmark star to quickly add the bookmark.
  2. The bookmark should be added with the current tab session.
    Result: Opening the bookmark in new tab should open it in the respective session.

add-using-star

Editing the bookmark

  1. Right click on any bookmark and click "Edit Bookmark".
  2. The edit bookmark dialog should open with the saved session selected.
  3. Change the selected session and save the bookmark.
    Result: Opening the bookmark in new tab should open in the selected session.

edit-bookmark

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

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
ryanml and others added 30 commits Mar 5, 2018
…there are no publishers in the ledger table.
Resolves #13754

Auditors:

Test Plan:
Adds s-s to the referral code
Block empty containers
Fix #13689

This bug was caused because menu items were using a hidden window for their new tabs.

We should _not_ use `electron{.remote}.BrowserWindow.getAllWindows()` or `.get{Active, Focused}Window` directly. There are two reasons that could have bad results:
1. We create BrowserWindows which are not normal tabbed renderer windows (e.g. some code that runs on Bookmark creation)
2. We have a hidden window sometimes (the Buffer Window)
Instead call `app/browser/window.js`: `getAllRendererWindows` and `getActiveWindowId`.
Fix for main menu actions, which can be called when there are no windows, but when searching for `BrowserWindow.`

Test Plan:
- Check all menu items open new tab in currently active window
- Check all menu items open new tab in currently active window, when app is not focused
- Check all menu items open new tab in new window if there is no visible window
- Check tabs in all windows are persisted on restart when app / windows close
- Check process quits in Windows when last visible window is closed
Menu items which need a window will create / show one if needed
Set min-width of URL bar to 30px, so URL bar does not shrink
too much and URL bar elements are not layered on each other
when window width is reduced.

Fix #13773
Resolves #4310

Auditors:

Test Plan:
Cezar Augusto
Fix #13695

Remove usage of textCalc as it was the last usage of tabs.executeScriptInBackground which is not compatible with upcoming muon v6.x since as it incorrectly used executeScriptInTab which should only work on a WebContents which is a Tab, not a Window. Muon does not have a generic WebContents.executeJavascript method.

Also added bonus of not requiring creating new Windows in order to calculate text width and having to manage when they are closed, etc which should address the following issues, if that was the cause:
Fix #13422
Fix #13277
Resolves #12271

Auditors:

Test Plan:
Calculate bookmarks bar overflow using css
fix #12671

test plan:
1. download an image and name it to rabbits.jpg
2. in the rabbits.jpg directory, start a localhost server: 'python -m SimpleHTTPServer 8000'
3. go to https://jsfiddle.net/c6y5qx5m/. you should see either 2 or
   3 copies of rabbits.jpg loaded.
4. go to about:preferences#security and enable 'Application Firewall'
5. go to https://jsfiddle.net/c6y5qx5m/ in a new private or session tab
   (to avoid loading cached files). now none of the rabbits.jpg images
   should load.
add off-by-default application firewall feature
fix #13668

Test Plan:
1. go to about:preferences#advanced
2. at the bottom, it should show a webrtc policy select menu which defaults
   to 'default'
3. turn on fingerprinting protection to 'block all'
4. go to https://browserleaks.com/webrtc. it should not show any IPs
5. turn off fingerprinting protection on that page. now it should show IPs
6. in about:preferences#advanced, set webrtc policy to 'default public
   interface only'
7. reload https://browserleaks.com/webrtc. it should only show the
   public IP.
8. in about:preferences#advanced, set webrtc policy to 'disable
   non-proxied UDP'
9. reload https://browserleaks.com/webrtc. it should show no IPs.
10. in about:preferences#advanced, set webrtc policy to 'default public
    and private interfaces'
11. reload https://browserleaks.com/webrtc. it should show both IPs.
Resolves #13531

Auditors:

Test Plan:
Reconcile is no longer triggered if there are no publishers in the ledger table.
Resolves #12792

Auditors:

Test Plan:
Adds PDF's to the ledger table
Fixes formatting for YT on shields page
add advanced webrtc IP handling preference
Fix #13802
Release notes for 0.22.x Release 2
fix #13805

Test Plan:
1. go to about:preferences > Advanced
2. you shouldn't see webrtc options there
3. go to about:preferences > security
4. you should see the webrtc options
Move webrtc options from advanced to security prefs
Fix #13802
Release notes for 0.22.x Release 2
NejcZdovc and others added 18 commits May 13, 2018
This reverts commit 12d502f, reversing
changes made to 0abdd64.
Revert "Merge pull request #13726 from NejcZdovc/fix/#13721-sort"
Fixes #12937

Auditors: @hasufell, @NejcZdovc
Update documentation for `Running Brave`
Fixes #12636

Auditors: @cezaraugusto

Test Plan:
1. Merge code
2. Re-push latest beta (`browser-laptop-release-linux` job in linux with `beta` param)
3. Re-push latest release (`browser-laptop-release-linux` job in linux with `release` param)
4. Verify packages are installable on Debian Buster
Add support for Debian Buster
Resolves #13721

Auditors:

Test Plan:
fix: Ensure that only "What do these policies mean" is clickable and the entire row
Fixed recovery overlay showing invalid amount before completion
Change facebook top tile to github and improve styling of top tiles icons
Fixes sortTable default sort
address #8956
-
this changes the options order from
allow/deny to deny/allow to match other notifcations
autoplay notification should follow spec
Travis job optimisations
@ishanrp ishanrp changed the title Feature/bookmark session Fixes #7417 Adds a session selector to the add/edit bookmark dialog May 20, 2018
@ishanrp ishanrp changed the title Fixes #7417 Adds a session selector to the add/edit bookmark dialog Fixes #7417 Adds a session selector to bookmark dialog May 20, 2018
@ishanrp ishanrp changed the title Fixes #7417 Adds a session selector to bookmark dialog Adds a session selector to bookmark dialog May 20, 2018
@bsclifton bsclifton added this to the Completed work milestone Aug 13, 2018
@bsclifton bsclifton requested review from NejcZdovc, bsclifton and ryanml Aug 13, 2018
@bsclifton bsclifton removed this from the Completed work milestone Sep 10, 2018
@NejcZdovc NejcZdovc removed their request for review Sep 18, 2018
@bsclifton bsclifton force-pushed the brave:master branch from be34a5d to a0cd488 Sep 18, 2018
@bsclifton
Copy link
Member

bsclifton commented Sep 21, 2018

Thanks for the patch, @ishanrp! Sorry that we weren't able to review this in a timely manner ☹️ I'm going to close this PR out as I've migrated the issue to our new code-base (brave-core). If you haven't checked out that repo yet, please give it a look 😄 We also have a build available here:
https://brave.com/download-dev

@bsclifton bsclifton closed this Sep 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.