Issue 1564-Fix FilterEngine::Matches for allowing request which is whitelisted in the ascendant node

src/FilterEngine.cpp

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 256 matching lines @@
 
 AdblockPlus::FilterPtr FilterEngine::Matches(const std::string& url,
     ContentType contentType,
     const std::vector<std::string>& documentUrls) const
 {
   if (documentUrls.empty())
     return CheckFilterMatch(url, contentType, "");
 
   std::string lastDocumentUrl = documentUrls.front();
   for (std::vector<std::string>::const_iterator it = documentUrls.begin();
-       it != documentUrls.end(); it++) {
-    const std::string documentUrl = *it;
-    AdblockPlus::FilterPtr match = CheckFilterMatch(documentUrl,
-                                                    CONTENT_TYPE_DOCUMENT,
-                                                    lastDocumentUrl);
+       it != documentUrls.end(); ++it)
+  {
+    lastDocumentUrl = *it;
+    AdblockPlus::FilterPtr match = CheckFilterMatch(url, contentType,
+      lastDocumentUrl);

[Oleksandr, 2014/11/13 13:32:18]

I would add a comment before CheckFilterMatch something like:
// Check if this request is whitelisted from any of the parent documents

[sergei, 2014/11/13 13:58:13]

added

     if (match && match->GetType() == AdblockPlus::Filter::TYPE_EXCEPTION)
-      return match;
-    lastDocumentUrl = documentUrl;
+        return match;
+    // We need to start from the same url for the filters like
+    // @@||example.org^$document, because "document — the page itself", so 
+    // we whitelist the everything which is requested by the page example.org.

[Oleksandr, 2014/11/13 13:32:18]

This comment is confusing, I would remove it.

[sergei, 2014/11/13 13:58:13]

See the updated version. It says ''parent'' but we start from the same page `ii
= it` not `ii = it + 1`. I think it looks like a bug and it's important to
comment that fact because without such definition of ''document'' type I would
expect to start really from parent.

+    for (std::vector<std::string>::const_iterator ii = it;
+      ii != documentUrls.end(); ++ii)
+    {
+      const std::string& documentUrl = *ii;
+      AdblockPlus::FilterPtr match = CheckFilterMatch(documentUrl,
+        CONTENT_TYPE_DOCUMENT, lastDocumentUrl);

[Oleksandr, 2014/11/13 13:32:18]

Before this CheckFilterMatch I'll add a comment like:
// Check if any of the parent documents are whitelisted

[sergei, 2014/11/13 13:58:13]

add before the for loop

+      if (match && match->GetType() == AdblockPlus::Filter::TYPE_EXCEPTION)
+        return match;
+    }
   }
 
   return CheckFilterMatch(url, contentType, lastDocumentUrl);
 }
 
 AdblockPlus::FilterPtr FilterEngine::CheckFilterMatch(const std::string& url,
     ContentType contentType,
     const std::string& documentUrl) const
 {
   JsValuePtr func = jsEngine->Evaluate("API.checkFilterMatch");
@@ skipping 108 matching lines @@
 }
 
 int FilterEngine::CompareVersions(const std::string& v1, const std::string& v2)
 {
   JsValueList params;
   params.push_back(jsEngine->NewValue(v1));
   params.push_back(jsEngine->NewValue(v2));
   JsValuePtr func = jsEngine->Evaluate("API.compareVersions");
   return func->Call(params)->AsInt();
 }