Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: chrome/content/elemHideEmulation.js

Issue 29464703: Issue 5313 - Make error reporting more robust. (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Also fixed a comment about parseSelector() that was wrong. Created June 14, 2017, 2:52 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/content/elemHideEmulation.js
===================================================================
--- a/chrome/content/elemHideEmulation.js
+++ b/chrome/content/elemHideEmulation.js
@@ -125,71 +125,16 @@
}
}
if (parens > 0)
return null;
return {text: content.substring(startIndex, i), end: i};
}
-/** Parse the selector
- * @param {string} selector the selector to parse
- * @return {Object} selectors is an array of objects,
- * or null in case of errors. hide is true if we'll hide
- * elements instead of styles..
- */
-function parseSelector(selector)
-{
- if (selector.length == 0)
- return [];
-
- let match = abpSelectorRegexp.exec(selector);
- if (!match)
- return [new PlainSelector(selector)];
-
- let selectors = [];
- if (match.index > 0)
- selectors.push(new PlainSelector(selector.substr(0, match.index)));
-
- let startIndex = match.index + match[0].length;
- let content = parseSelectorContent(selector, startIndex);
- if (!content)
- {
- reportError(new SyntaxError("Failed to parse Adblock Plus " +
- `selector ${selector}, ` +
- "due to unmatched parentheses."));
- return null;
- }
- if (match[1] == "properties")
- selectors.push(new PropsSelector(content.text));
- else if (match[1] == "has")
- {
- let hasSelector = new HasSelector(content.text);
- if (!hasSelector.valid())
- return null;
- selectors.push(hasSelector);
- }
- else
- {
- // this is an error, can't parse selector.
- reportError(new SyntaxError("Failed to parse Adblock Plus " +
- `selector ${selector}, invalid ` +
- `pseudo-class :-abp-${match[1]}().`));
- return null;
- }
-
- let suffix = parseSelector(selector.substr(content.end + 1));
- if (suffix == null)
- return null;
-
- selectors.push(...suffix);
-
- return selectors;
-}
-
/** Stringified style objects
* @typedef {Object} StringifiedStyle
* @property {string} style CSS style represented by a string.
* @property {string[]} subSelectors selectors the CSS properties apply to.
*/
/**
* Produce a string representation of the stylesheet entry.
@@ -241,29 +186,24 @@
*getSelectors(prefix, subtree, styles)
{
yield [prefix + this._selector, subtree];
}
};
const incompletePrefixRegexp = /[\s>+~]$/;
-function HasSelector(selector)
+function HasSelector(selectors)
{
- this._innerSelectors = parseSelector(selector);
+ this._innerSelectors = selectors;
}
HasSelector.prototype = {
requiresHiding: true,
- valid()
- {
- return this._innerSelectors != null;
- },
-
*getSelectors(prefix, subtree, styles)
{
for (let element of this.getElements(prefix, subtree, styles))
yield [makeSelector(element, ""), element];
},
/**
* Generator function returning selected elements.
@@ -339,16 +279,73 @@
}
catch (e)
{
// Invalid URL, assume that it is first-party.
return true;
}
},
+ /** Parse the selector
+ * @param {string} selector the selector to parse
+ * @return {Array} selectors is an array of objects,
+ * or null in case of errors.
+ */
+ parseSelector(selector)
+ {
+ if (selector.length == 0)
+ return [];
+
+ let match = abpSelectorRegexp.exec(selector);
+ if (!match)
+ return [new PlainSelector(selector)];
+
+ let selectors = [];
+ if (match.index > 0)
+ selectors.push(new PlainSelector(selector.substr(0, match.index)));
+
+ let startIndex = match.index + match[0].length;
+ let content = parseSelectorContent(selector, startIndex);
+ if (!content)
+ {
+ this.window.console.error(
+ new SyntaxError("Failed to parse Adblock Plus " +
+ `selector ${selector}, ` +
Wladimir Palant 2017/06/19 11:22:37 Nit: There should be no comma here, I vaguely reme
hub 2017/06/19 13:24:42 Done.
+ "due to unmatched parentheses."));
+ return null;
+ }
+ if (match[1] == "properties")
+ selectors.push(new PropsSelector(content.text));
+ else if (match[1] == "has")
+ {
+ let hasSelectors = this.parseSelector(content.text);
+ if (hasSelectors == null)
+ return null;
+ let hasSelector = new HasSelector(hasSelectors);
+ selectors.push(hasSelector);
Wladimir Palant 2017/06/19 11:22:37 Nit: The temporary hasSelector variable no longer
hub 2017/06/19 13:24:42 Done.
+ }
+ else
+ {
+ // this is an error, can't parse selector.
+ this.window.console.error(
+ new SyntaxError("Failed to parse Adblock Plus " +
+ `selector ${selector}, invalid ` +
+ `pseudo-class :-abp-${match[1]}().`));
+ return null;
+ }
+
+ let suffix = this.parseSelector(selector.substr(content.end + 1));
+ if (suffix == null)
+ return null;
+
+ selectors.push(...suffix);
+
+ return selectors;
+ },
+
addSelectors(stylesheets)
{
let selectors = [];
let selectorFilters = [];
let elements = [];
let elementFilters = [];
@@ -406,29 +403,25 @@
if (stylesheet)
this.addSelectors([stylesheet]);
},
apply()
{
this.getFiltersFunc(patterns =>
{
- let oldReportError = reportError;
- reportError = error => this.window.console.error(error);
-
this.patterns = [];
for (let pattern of patterns)
{
- let selectors = parseSelector(pattern.selector);
+ let selectors = this.parseSelector(pattern.selector);
if (selectors != null && selectors.length > 0)
this.patterns.push({selectors, text: pattern.text});
}
if (this.patterns.length > 0)
{
let {document} = this.window;
this.addSelectors(document.styleSheets);
document.addEventListener("load", this.onLoad.bind(this), true);
}
- reportError = oldReportError;
});
}
};
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld