Adding return after rejects in a few places. Implementing Ghostery side of GH-885. Fix for GH-871. #10
Conversation
zarembsky
commented
Mar 21, 2018
•
|
zarembsky
changed the title
zarembsky
changed the title
zarembsky
changed the title
| if (bug_id) { | ||
| app_id = bugDb.db.bugs[bug_id].aid; | ||
| cat_id = bugDb.db.apps[app_id].cat; | ||
| incognito = tabInfo.getTabInfo(tab_id, 'incognito'); | ||
| tab_host = tabInfo.getTabInfo(tab_id, 'host'); | ||
| fromRedirect = globals.REDIRECT_MAP.has(request_id); | ||
| block = this._checkBlocking(app_id, cat_id, tab_id, tab_host, page_url, request_id); | ||
| const retVal = this._checkBlocking(app_id, cat_id, tab_id, tab_host, page_url, request_id); |
christophertino
Mar 23, 2018
Member
const {block, ss_unblock} = this._checkBlocking(app_id, cat_id, tab_id, tab_host, page_url, request_id);
const {block, ss_unblock} = this._checkBlocking(app_id, cat_id, tab_id, tab_host, page_url, request_id);
zarembsky
Mar 23, 2018
•
Author
Contributor
The problem with the code const {block, ss_unblock} = is that block should be visible out of scope if(bug_id) { }. We can get away with extra assignment, like:
const {block1, ss_unblock} = ...
block = block1;
The problem with the code const {block, ss_unblock} = is that block should be visible out of scope if(bug_id) { }. We can get away with extra assignment, like:
const {block1, ss_unblock} = ...
block = block1;
| * @param {number} request_id request id | ||
| * @return {boolean} | ||
| * @return {Object} {block, ss_unblock} | ||
| */ | ||
| _checkBlocking(app_id, cat_id, tab_id, tab_host, page_url, request_id) { |
christophertino
Mar 23, 2018
Member
this should have an object return, correct? looks to just be returning block at the moment, instead of return {block:true, ss_unblock:false} etc
See line 793
this should have an object return, correct? looks to just be returning block at the moment, instead of return {block:true, ss_unblock:false} etc
See line 793
zarembsky
Mar 23, 2018
Author
Contributor
block IS object, as it is returned by shouldBlock, which returns object.
block IS object, as it is returned by shouldBlock, which returns object.
| */ | ||
| shouldBlock(app_id, cat_id, tab_id, tab_host, tab_url) { | ||
| if (globals.SESSION.paused_blocking) { | ||
| return false; | ||
| return { block: false }; |
christophertino
Mar 23, 2018
Member
we should return {block, ss_unblock} in all cases to prevent undefined errors
we should return {block, ss_unblock} in all cases to prevent undefined errors
zarembsky
Mar 23, 2018
Author
Contributor
You mean if we ever call shouldBlock elsewhere (but we don't)
You mean if we ever call shouldBlock elsewhere (but we don't)
zarembsky
Mar 23, 2018
•
Author
Contributor
The reason I did not go with {{block, ss_unblock} was that it is performance-critical, and most of the time ss_unblock is not needed.
I'll make the requested changes though.
The reason I did not go with {{block, ss_unblock} was that it is performance-critical, and most of the time ss_unblock is not needed.
I'll make the requested changes though.
