Using scroll-touch-edge to record the distance #15128

darkdh · 2018-08-30T01:00:04Z

cause wheel event will only fire once each scroll since C69

mainly
fix #15114

also
fix #14871
fix #14829
fix #14785

Auditor: @bsclifton

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:

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

Update master to 0.25.0

Fix some error branches in tor controller logic.

@diracdeltas

This should address the failure reported in: #14675 (comment) fix #14630 Auditors: @diracdeltas Test Plan: see #14630

This, and TorControl.destroy(), are worth making idempotent because errors may happen synchronously _or_ asynchronously, so there are two levels at which a caller might want to clean up. For example, authentication failure is reported as a 515 error from tor, _and_ the tor daemon will close the control connection leading to an asynchronous error.

There is no way to know exactly when this might happen, alas: readables and writables don't necessarily emit their close events promptly.

for some reason, the windows path has an extra `../` prefix compared to mac so the favicon doesn't load. this works around the issue by removing a `../` on windows. Test Plan: 1. open new tab 2. hit `:g` in the urlbar 3. the search favicon should load on all platforms

…es-v2 Fix some more error branches, assert sanity, and automatically test it all.

This was always signing the RPM because the test was failing, now we will test and if the signature exists then move on. The error seen was: tools/upload_to_rpm_repo: line 18: !rpm: command not found

fix search favicon path on windows

@diracdeltas

…ut key for new private tab Fixes #14504 Auditors: @diracdeltas

Fix #14680 Test Plan: 1. open brave, turn on showing bookmarks toolbar and showing home button in about:preferences. bookmark some sites. 2. open tor tab and try to click on bookmarks before tor finishes loading. also try clicking on the home button and clicking the History > Home menu item. 3. these loads should be blocked until tor finishes initializing

Add shortcut for private tab with Tor and also update existing shortc…

Allow upload script to check if current signature exists.

Fixes #14647

Block tab.loadURL in tor tabs until tor is initialized

fix #14653 for some reason, renaming the searx favicon to something that doesn't have 'searx' in the filename makes it reappear Test Plan: 1. make a packaged build 2. open brave 3. hit ':x' in urlbar. searx favicon should appear.

Fix mysterious searx favicon disappearing in builds

@darkdh

Fixes #14719 Auditors: @darkdh

Update to Muon 7.1.6

To do this, we separate step of killing the daemon and control channel from the step of ceasing to watch for updates. On errors, we want to kill the daemon and control channel, but continue watching for updates in case the daemon restarts. Possible snag: we do not rate-limit failures if they are recursive. Not currently an issue but we should take care of this in the future.

@diracdeltas

fix #14431 Auditors: @diracdeltas @bsclifton Test Plan: I: 1. Open a tab _without_ Tor (private or nonprivate). 2. Enter: https://nyttips4bmquxfzw.onion/ 3. Confirm that Brave blocks loading the URL. II: 1. Open a tab _without_ Tor (private or nonprivate). 2. Enter: https://nyttips4bmquxfzw.onion:12345/ 3. Confirm that Brave blocks loading the URL. III: 1. Open a private tab with Tor. 2. Enter: https://nyttips4bmquxfzw.onion/ 3. Confirm that the NYT SecureDrop page loads. 4. Bookmark it. 5. Open a tab _without_ Tor (private or nonprivate). 6. Try to load the bookmark. 7. Confirm that Brave blocks loading the bookmark.

Don't emit an 'error' event if we did pass them synchronously. Establish an 'error' handler in filtering.js for asynchronous errors.

Removes delay for referral

fix #15074 Test Plan: DM'ed

block chrome://brave URL loads

@diracdeltas

Fixes #15075 Auditors: @diracdeltas

Allow `view-source` as openable by context menu

Resolves #15073 Auditors: Test Plan:

Removes shadow

fix typo in test description: deafult -> default

fix typo in js extension comment: occuring -> occurring

Fix #15100

Release notes for 0.23.x Release 5

@darkdh

- has fix for extension breakage - has crash fix Fixes #15119 Auditors: @darkdh

Update to Muon 8.0.9

@bsclifton

only fire once each scroll since C69 mainly fix #15114 also fix #14871 fix #14829 fix #14785 Auditor: @bsclifton

darkdh · 2018-08-30T01:01:08Z

don't merge it until master has C69 muon

joelwass · 2018-08-31T21:07:42Z

I believe this logic is broken. If I swipe right on the trackpad with the intention of navigating backwards, 1) the sensitivity is incredibly high, but 2) if i change directions on horizontal scroll and try to "change my mind" and scroll back the other way, your deltaX keeps increasing since I haven't lifted my fingers

joelwass · 2018-08-31T21:11:34Z

I can go ahead and create some changes on top of this PR if you'd like?

darkdh · 2018-08-31T21:13:56Z

no, this is because require chromium 69 which is in 8.1.x. So we have to wait for master upgrading muon version so we can test this PR and merge it

darkdh · 2018-08-31T21:14:32Z

for now, you can try 0.23.x-c69 branch. And create a PR on top of it if you want

joelwass · 2018-08-31T21:24:29Z

app/renderer/components/main/main.js

+ } else {
+ appActions.swipedRight(percent)
+ }
+ deltaX += 1


what's the intended purpose of this line and line 276? that seems to cause the issue i mentioned in the discussion

again, that is intentional for features::kTouchpadAndWheelScrollLatching enabled in c69 and master does't have the compatible version of muon now.
could you try 0.23.x-c69 branch first?

yea, still broken off of 0.23.x-c69 branch

did you delete node_modules/electron-prebuilt and redo a npm install?
If you didn't, that means you don't have the correct muon version.

you can also try https://github.com/brave/browser-laptop/releases/tag/v0.23.202beta which is just built from that branch and contains this fix

yes i rm rf'd node modules on the 0.23.x-c69 branch and the issue still occurs. Also the issue i mentioned still occurs in https://github.com/brave/browser-laptop/releases/tag/v0.23.202beta

would you like me to try to fix?

I believe the problem you talked about is no way to cancel the navigation once you reach 100%. you can try to fix that. But I don't see so called incredibly high sensitivity

I'm going to emit more attributes from muon for this event so I can update the precise units

you can try 0.23.x-c69 again now. new muon build is ready

@bsclifton

depends on brave/muon@b9cec60 Auditors: @bsclifton

bsclifton · 2018-10-19T06:47:59Z

Closing as wontfix - this should be fixed in the new version of Brave:
https://brave.com/download/

bsclifton and others added 30 commits July 10, 2018 00:14

Update master to 0.25.0

75006a7

Merge pull request #14696 from bsclifton/master

811328e

Update master to 0.25.0

Merge pull request #14675 from brave/riastradh-tor-controlerrorbranches

e680e1c

Fix some error branches in tor controller logic.

Out of paranoia, check we're closing the right control channel.

0899642

Allow TorControl.destroy to be idempotent.

2a30a9b

This should address the failure reported in: #14675 (comment) fix #14630 Auditors: @diracdeltas Test Plan: see #14630

Emit an error event if authentication fails.

be5453e

Don't assert that we have closed.

7499ae4

There is no way to know exactly when this might happen, alas: readables and writables don't necessarily emit their close events promptly.

Automatically test tor launch errors.

44d3aba

Merge pull request #14703 from brave/riastradh-tor-controlerrorbranch…

a742002

…es-v2 Fix some more error branches, assert sanity, and automatically test it all.

Allow upload script to check if current signature exists.

7af0146

This was always signing the RPM because the test was failing, now we will test and if the signature exists then move on. The error seen was: tools/upload_to_rpm_repo: line 18: !rpm: command not found

Merge pull request #14704 from brave/fix/windows-favicon-path

3d830d7

fix search favicon path on windows

Add shortcut for private tab with Tor and also update existing shortc…

c265416

…ut key for new private tab Fixes #14504 Auditors: @diracdeltas

fix test to use return code not command output

24a5b7a

Merge pull request #14711 from bsclifton/tor-tab-shortcut

050aa3c

Add shortcut for private tab with Tor and also update existing shortc…

Merge pull request #14705 from brave/upload_to_rpm_repo

e78fc93

Allow upload script to check if current signature exists.

Allow for a per-locale default search engine (value comes from config)

bfdba0e

Fixes #14647

Merge pull request #14709 from brave/fix/tor-bookmarks

1cf53e1

Block tab.loadURL in tor tabs until tor is initialized

Fix mysterious searx favicon disappearing in builds

bb20ceb

fix #14653 for some reason, renaming the searx favicon to something that doesn't have 'searx' in the filename makes it reappear Test Plan: 1. make a packaged build 2. open brave 3. hit ':x' in urlbar. searx favicon should appear.

Merge pull request #14718 from brave/fix/searx-favicon

4c7e194

Fix mysterious searx favicon disappearing in builds

Update to Muon 7.1.6

24e726c

Fixes #14719 Auditors: @darkdh

Merge pull request #14720 from bsclifton/update-muon-7.1.6

794213b

Update to Muon 7.1.6

Don't spin polling unless the watcher suggests we retry again.

487bf48

Pass control channel errors synchronously, if commands are waiting.

950368c

Don't emit an 'error' event if we did pass them synchronously. Establish an 'error' handler in filtering.js for asynchronous errors.

Omit now-duplicated log message.

2f40572

Omit needless `tor:' tags on error messages.

79794d6

bsclifton and others added 16 commits August 21, 2018 12:14

Merge pull request #15068 from NejcZdovc/hotfix/#15067-delay

00aaee7

Removes delay for referral

block chrome://brave URL loads

09d2453

fix #15074 Test Plan: DM'ed

Merge pull request #15076 from brave/fix/15074

9d88931

block chrome://brave URL loads

Allow view-source as openable by context menu

6477fde

Fixes #15075 Auditors: @diracdeltas

Merge pull request #15078 from bsclifton/fix-view-source

75b8cda

Allow `view-source` as openable by context menu

Removes shadow

c0dea4e

Resolves #15073 Auditors: Test Plan:

Merge pull request #15077 from NejcZdovc/fix/#15073-solve

6d1df8b

Removes shadow

fix typo in js extension comment: occuring -> occurring

af156c5

fix typo in test description: deafult -> default

9996936

Merge pull request #15093 from nikolas/patch-2

ceec9bf

fix typo in test description: deafult -> default

Merge pull request #15092 from nikolas/patch-1

a1e0a78

fix typo in js extension comment: occuring -> occurring

Release notes for 0.23.x Release 5

7973472

Fix #15100

Merge pull request #15105 from brave/release-notes-0.23.x-Release5

a57124b

Release notes for 0.23.x Release 5

Update to Muon 8.0.9

8d679f0

- has fix for extension breakage - has crash fix Fixes #15119 Auditors: @darkdh

Merge pull request #15120 from bsclifton/update-muon-8.0.9

8e09743

Update to Muon 8.0.9

Using scroll-touch-edge to record the distance cause wheel event will

c46f54c

only fire once each scroll since C69 mainly fix #15114 also fix #14871 fix #14829 fix #14785 Auditor: @bsclifton

darkdh self-assigned this Aug 30, 2018

darkdh requested a review from bsclifton August 30, 2018 01:00

darkdh added the cr69 label Aug 30, 2018

joelwass reviewed Aug 31, 2018

View reviewed changes

Obtain deltaX from scroll-touch-edge

619b7a4

depends on brave/muon@b9cec60 Auditors: @bsclifton

bsclifton force-pushed the master branch from be34a5d to a0cd488 Compare September 18, 2018 17:39

bsclifton closed this Oct 19, 2018

bsclifton deleted the hoz-swipe-nav branch October 19, 2018 06:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using scroll-touch-edge to record the distance #15128

Using scroll-touch-edge to record the distance #15128

darkdh commented Aug 30, 2018

darkdh commented Aug 30, 2018

joelwass commented Aug 31, 2018

joelwass commented Aug 31, 2018

darkdh commented Aug 31, 2018

darkdh commented Aug 31, 2018

joelwass Aug 31, 2018

darkdh Aug 31, 2018

joelwass Sep 1, 2018

darkdh Sep 1, 2018

darkdh Sep 1, 2018

joelwass Sep 2, 2018

darkdh Sep 2, 2018

darkdh Sep 2, 2018

darkdh Sep 2, 2018

bsclifton commented Oct 19, 2018

Using scroll-touch-edge to record the distance #15128

Using scroll-touch-edge to record the distance #15128

Conversation

darkdh commented Aug 30, 2018

Submitter Checklist:

Test Plan:

Reviewer Checklist:

darkdh commented Aug 30, 2018

joelwass commented Aug 31, 2018

joelwass commented Aug 31, 2018

darkdh commented Aug 31, 2018

darkdh commented Aug 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsclifton commented Oct 19, 2018