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

Unified Diff: include.preload.js

Issue 29575739: Issue 5864 - Remove previous style sheet before adding one (Closed) Base URL: https://hg.adblockplus.org/adblockpluschrome/
Patch Set: Inline replaceStyleSheet Created Oct. 18, 2017, 1:41 p.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 | lib/cssInjection.js » ('j') | lib/cssInjection.js » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: include.preload.js
===================================================================
--- a/include.preload.js
+++ b/include.preload.js
@@ -337,19 +337,19 @@
this.observer.disconnect();
clearTimeout(this.timeout);
}
};
function ElemHide()
{
this.shadow = this.createShadowTree();
- this.style = null;
+ this.styles = new Map();
this.tracer = null;
- this.inject = true;
+ this.inline = true;
kzar 2018/01/24 14:30:53 How come you renamed this one? (I'm not against re
Manish Jethani 2018/01/24 16:37:44 Yeah, this was for clarity [1] since we already us
kzar 2018/01/25 11:52:37 Acknowledged.
this.emulatedPatterns = null;
this.elemHideEmulation = new ElemHideEmulation(
this.addSelectors.bind(this),
this.hideElements.bind(this)
);
}
ElemHide.prototype = {
@@ -374,34 +374,47 @@
// avoid creating the shadowRoot twice.
let shadow = document.documentElement.shadowRoot ||
document.documentElement.createShadowRoot();
shadow.appendChild(document.createElement("shadow"));
return shadow;
},
- injectSelectors(selectors, filters)
+ addSelectorsInline(selectors, filters, groupName)
kzar 2018/01/24 14:30:53 Just me or is the `filters` parameter not used?
Manish Jethani 2018/01/24 16:37:44 True, removed.
{
- if (!this.style)
+ let style = this.styles.get(groupName);
+
+ if (style)
+ {
+ while (style.sheet.cssRules.length > 0)
+ style.sheet.deleteRule(0);
+ }
+
+ if (selectors.length == 0)
+ return;
+
+ if (!style)
{
// Create <style> element lazily, only if we add styles. Add it to
// the shadow DOM if possible. Otherwise fallback to the <head> or
// <html> element. If we have injected a style element before that
// has been removed (the sheet property is null), create a new one.
- this.style = document.createElement("style");
+ style = document.createElement("style");
(this.shadow || document.head ||
- document.documentElement).appendChild(this.style);
+ document.documentElement).appendChild(style);
// It can happen that the frame already navigated to a different
// document while we were waiting for the background page to respond.
// In that case the sheet property will stay null, after addind the
// <style> element to the shadow DOM.
- if (!this.style.sheet)
+ if (!style.sheet)
return;
+
+ this.styles.set(groupName, style);
}
// If using shadow DOM, we have to add the ::content pseudo-element
// before each selector, in order to match elements within the
// insertion point.
let preparedSelectors = [];
if (this.shadow)
{
@@ -429,37 +442,35 @@
).join(", ");
this.style.sheet.insertRule(selector + "{display: none !important;}",
this.style.sheet.cssRules.length);
}
},
addSelectors(selectors, filters)
{
- if (selectors.length == 0)
- return;
-
- if (this.inject)
+ if (this.inline)
{
// Insert the style rules inline if we have been instructed by the
// background page to do so. This is usually the case, except on platforms
// that do support user stylesheets via the browser.tabs.insertCSS API
// (Firefox 53 onwards for now and possibly Chrome in the near future).
// Once all supported platforms have implemented this API, we can remove
// the code below. See issue #5090.
// Related Chrome and Firefox issues:
// https://bugs.chromium.org/p/chromium/issues/detail?id=632009
// https://bugzilla.mozilla.org/show_bug.cgi?id=1310026
- this.injectSelectors(selectors, filters);
+ this.addSelectorsInline(selectors, filters, "emulated");
}
else
{
browser.runtime.sendMessage({
type: "elemhide.injectSelectors",
- selectors
+ selectors,
+ groupName: "emulated"
});
}
if (this.tracer)
this.tracer.addSelectors(selectors, filters);
},
hideElements(elements, filters)
@@ -480,28 +491,25 @@
apply()
{
browser.runtime.sendMessage({type: "elemhide.getSelectors"}, response =>
{
if (this.tracer)
this.tracer.disconnect();
this.tracer = null;
- if (this.style && this.style.parentElement)
- this.style.parentElement.removeChild(this.style);
- this.style = null;
-
if (response.trace)
this.tracer = new ElementHidingTracer();
- this.inject = response.inject;
+ this.inline = response.inline;
- if (this.inject)
- this.addSelectors(response.selectors);
- else if (this.tracer)
kzar 2018/01/24 14:30:53 This logic seems different now, did you change it
Manish Jethani 2018/01/24 16:37:44 Yes, so there's a new function now called addSelec
kzar 2018/01/25 11:52:37 Acknowledged.
+ if (this.inline)
+ this.addSelectorsInline(response.selectors);
kzar 2018/01/24 14:30:53 Shouldn't we pass the other parameters (well `grou
Manish Jethani 2018/01/24 16:37:44 Yeah so the default group is undefined. We could j
kzar 2018/01/25 11:52:37 Yea, I think making the name here explicit would m
Manish Jethani 2018/01/25 14:29:33 OK, I'm using the name "standard" here since these
+
+ if (this.tracer)
this.tracer.addSelectors(response.selectors);
this.elemHideEmulation.apply(response.emulatedPatterns);
});
}
};
if (document instanceof HTMLDocument)
« no previous file with comments | « no previous file | lib/cssInjection.js » ('j') | lib/cssInjection.js » ('J')

Powered by Google App Engine
This is Rietveld