Issue 4659 - Incorporate element hiding emulation filters

Issue 4659 - Incorporate element hiding emulation filters

[Felix Dahlke, 2016-12-13 21:27:27]

https://codereview.adblockplus.org/29367475/diff/29367476/metadata.common
File metadata.common (right):

https://codereview.adblockplus.org/29367475/diff/29367476/metadata.common#new...
metadata.common:31: core-common.js = adblockpluscore/lib/common.js
common.js was already taken for adblockplusui/common.js, but I'm not very
attached to the `core-` prefix I went for here.

[kzar, 2016-12-19 13:51:40]

https://codereview.adblockplus.org/29367475/diff/29367476/dependencies
File dependencies (right):

https://codereview.adblockplus.org/29367475/diff/29367476/dependencies#newcode6
dependencies:6: adblockplusui = adblockplusui hg:4286b54830e5 git:dd8888e
Mind updating to the latest revision of adblockplusui whilst at it? (That will
save me a job since I've made a change for the devtools panel since but was not
able to update the dependency until these changes landed.)

https://codereview.adblockplus.org/29367475/diff/29367476/metadata.common
File metadata.common (right):

https://codereview.adblockplus.org/29367475/diff/29367476/metadata.common#new...
metadata.common:31: core-common.js = adblockpluscore/lib/common.js
On 2016/12/13 21:27:27, Felix Dahlke wrote:
> common.js was already taken for adblockplusui/common.js, but I'm not very
> attached to the `core-` prefix I went for here.

Mind instead using the convert_js section to combine
adblockpluscore/lib/common.js and
adblockpluscore/chrome/content/elemHideEmulation.js into one file? You could
avoid the core-common.js name that way. Also it avoids us having yet another
content script. (Some users complain of the latency Adblock Plus adds to page
loads and IIRC those are worsened by having multiple content scripts.)

[Felix Dahlke, 2016-12-20 14:35:43]

New patch set is up.

https://codereview.adblockplus.org/29367475/diff/29367476/dependencies
File dependencies (right):

https://codereview.adblockplus.org/29367475/diff/29367476/dependencies#newcode6
dependencies:6: adblockplusui = adblockplusui hg:4286b54830e5 git:dd8888e
On 2016/12/19 13:51:40, kzar wrote:
> Mind updating to the latest revision of adblockplusui whilst at it? (That will
> save me a job since I've made a change for the devtools panel since but was
not
> able to update the dependency until these changes landed.)

Done.

https://codereview.adblockplus.org/29367475/diff/29367476/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29367475/diff/29367476/include.preload.js#...
include.preload.js:18: var MutationObserver = window.MutationObserver ||
window.WebKitMutationObserver;
Caused by rebasing.

https://codereview.adblockplus.org/29367475/diff/29367476/include.preload.js#...
include.preload.js:167: if (MutationObserver)
Caused by rebasing.

https://codereview.adblockplus.org/29367475/diff/29367476/metadata.common
File metadata.common (right):

https://codereview.adblockplus.org/29367475/diff/29367476/metadata.common#new...
metadata.common:31: core-common.js = adblockpluscore/lib/common.js
On 2016/12/19 13:51:40, kzar wrote:
> On 2016/12/13 21:27:27, Felix Dahlke wrote:
> > common.js was already taken for adblockplusui/common.js, but I'm not very
> > attached to the `core-` prefix I went for here.
> 
> Mind instead using the convert_js section to combine
> adblockpluscore/lib/common.js and
> adblockpluscore/chrome/content/elemHideEmulation.js into one file? You could
> avoid the core-common.js name that way. Also it avoids us having yet another
> content script. (Some users complain of the latency Adblock Plus adds to page
> loads and IIRC those are worsened by having multiple content scripts.)

That's a cool idea, didn't occur to me at all. Done!

https://codereview.adblockplus.org/29367475/diff/29369415/metadata.common
File metadata.common (right):

https://codereview.adblockplus.org/29367475/diff/29369415/metadata.common#new...
metadata.common:3: backgroundScripts = ext/common.js
Caused by rebasing.

[kzar, 2016-12-20 15:08:33]

Otherwise LGTM. You want to have a look Sebastian?

https://codereview.adblockplus.org/29367475/diff/29369415/metadata.common
File metadata.common (right):

https://codereview.adblockplus.org/29367475/diff/29369415/metadata.common#new...
metadata.common:30: core-common.js = adblockpluscore/lib/common.js
Is this line still required?

[Felix Dahlke, 2016-12-20 16:19:03]

New patch set is up.

https://codereview.adblockplus.org/29367475/diff/29369415/metadata.common
File metadata.common (right):

https://codereview.adblockplus.org/29367475/diff/29369415/metadata.common#new...
metadata.common:30: core-common.js = adblockpluscore/lib/common.js
On 2016/12/20 15:08:33, kzar wrote:
> Is this line still required?

Done.

[Sebastian Noack, 2016-12-20 18:07:08]

LGTM