 Issue 29575739:
  Issue 5864 - Remove previous style sheet before adding one  (Closed) 
  Base URL: https://hg.adblockplus.org/adblockpluschrome/
    
  
    Issue 29575739:
  Issue 5864 - Remove previous style sheet before adding one  (Closed) 
  Base URL: https://hg.adblockplus.org/adblockpluschrome/| 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) |