-
Notifications
You must be signed in to change notification settings - Fork 974
Conversation
fix #10864 requires muon 4.3.19+
Codecov Report
@@ Coverage Diff @@
## master #10896 +/- ##
==========================================
- Coverage 54.14% 54.1% -0.04%
==========================================
Files 248 248
Lines 21751 21765 +14
Branches 3383 3388 +5
==========================================
- Hits 11777 11776 -1
- Misses 9974 9989 +15
|
app/filtering.js
Outdated
// Always allow fullscreen if setting is ON | ||
const alwaysAllowFullscreen = module.exports.alwaysAllowFullscreen() === fullscreenOption.ALWAYS_ALLOW | ||
if (permission === 'fullscreen' && alwaysAllowFullscreen) { | ||
response.push(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if permission === 'fullscreen'
, and both origin.startsWith('chrome-extension://' + config.torrentExtensionId)
and alwaysAllowFullscreen
are true, wouldn't you be calling response.push(true)
twice in this loop (since the conditional is if
, not else if
)? response[i] = ...
instead of response.push(...)
seems clearer
some of the permissions tests are failing but could be intermittent https://travis-ci.org/brave/browser-laptop/jobs/274266563 |
This addresses review feedback for #10864
@diracdeltas I pushed a follow up commit to address the feedback of making sure we push at most once per permission type. |
Sister PR for 4.3.x |
Already covered by this test: ``` 1) notificationBar permissions can accept permission request persistently: Promise was rejected with the following reason: timeout ``` Fix #10957
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still no notification for me going to https://browserleaks.com/geo. Running Muon 4.4.15. Am I missing something?
mentioned via slack DM but this is already reviewed and accepted in 4.3, it's just waiting for new builds so we can merge it to 4.4. |
handle an array of permission types
handle an array of permission types
fix #10864
requires muon 4.3.19+
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
Tests