-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Logger incorrectly shows all popups as blocked by one filter. #2776
Comments
Pretty sure it's a regression from 0232382#diff-ce215d94a52d449f1a34f292126608d7. |
@gorhill: Actually it still fails. I did this to clear diff --git a/src/js/tab.js b/src/js/tab.js
index 95003969..0c200211 100644
--- a/src/js/tab.js
+++ b/src/js/tab.js
@@ -639,6 +639,7 @@ vAPI.tabs.onPopupUpdated = (function() {
var mapPopunderResult = function(popunderURL, popunderHostname, result) {
if (
+ result === 0 ||
logData === undefined ||
logData.source !== 'static' ||
logData.token === µb.staticNetFilteringEngine.noTokenHash
@@ -650,11 +651,11 @@ vAPI.tabs.onPopupUpdated = (function() {
}
var re = new RegExp(logData.regex),
matches = re.exec(popunderURL);
- if ( matches === null ) { return ''; }
+ if ( matches === null ) { return 0; }
var beg = matches.index,
end = beg + matches[0].length,
pos = popunderURL.indexOf(popunderHostname);
- if ( pos === -1 ) { return ''; }
+ if ( pos === -1 ) { return 0; }
// https://github.com/gorhill/uBlock/issues/1471
// We test whether the opener hostname as at least one character
// within matched portion of URL.
@@ -704,9 +705,6 @@ vAPI.tabs.onPopupUpdated = (function() {
};
return function(targetTabId, openerTabId) {
- // https://github.com/gorhill/uBlock/issues/2776
- logData = undefined;
-
// Opener details.
var tabContext = µb.tabContextManager.lookup(openerTabId);
if ( tabContext === null ) { return; }
@@ -753,6 +751,10 @@ vAPI.tabs.onPopupUpdated = (function() {
// Log only for when there was a hit against an actual filter (allow or block).
if ( µb.logger.isEnabled() ) {
+ if ( result === 0 ) {
+ // https://github.com/gorhill/uBlock/issues/2776
+ logData = undefined;
+ }
µb.logger.writeOne(
popupType === 'popup' ? openerTabId : targetTabId,
'net', |
I think I'm still worried about this "second try" popunder match which will not be executed even when it should. You check Lines 690 to 692 in 7fb034f
mapPopunderResult() can return '' in some code path. This looks like unwanted change in logic from refactoring commit, and will return too early from the popunderMatch() function.
|
Why should it? A match has been found, no point trying to find another one. |
I'm talking about the case when the match is not found. The #1598 case. These both cases will return Line 653 in 7fb034f
Line 657 in 7fb034f
Lines 690 to 692 in 7fb034f
Before the changes in |
This is definitely wrong, I don't know how I missed this. This should be |
Damn, now I remember why I've added diff --git a/src/js/tab.js b/src/js/tab.js
index 1454cc16..1a385236 100644
--- a/src/js/tab.js
+++ b/src/js/tab.js
@@ -639,6 +639,7 @@ vAPI.tabs.onPopupUpdated = (function() {
var mapPopunderResult = function(popunderURL, popunderHostname, result) {
if (
+ result === 0 ||
logData === undefined ||
logData.source !== 'static' ||
logData.token === µb.staticNetFilteringEngine.noTokenHash When first call to Still the end result will be 0, because |
One or more specific URLs where the issue occurs
http://raymondhill.net/ublock/popup.html
Screenshot in which the issue can be seen
Steps for anyone to reproduce the issue
After first blocked popup every other popup that gets blocked or not will be shown as blocked by filter used to block first popup. See the screenshot where
about:blank
is not blocked and it is shown as blocked by the logger.Your settings
The text was updated successfully, but these errors were encountered: