Opened on 09/25/2017 at 01:23:45 PM
Closed on 09/28/2017 at 05:14:56 PM
#5773 closed defect (fixed)
Element hiding emulation filters do not work in old Chrome/Opera versions
| Reported by: | Ross | Assignee: | hfiguiere |
|---|---|---|---|
| Priority: | Unknown | Milestone: | Adblock-Plus-3.0-for-Firefox |
| Module: | Core | Keywords: | |
| Cc: | sebastian, hfiguiere, kzar, trev | Blocked By: | |
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | no | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): |
https://codereview.adblockplus.org/29555834 |
||
Description (last modified by Ross)
Environment
ABP 1.13.3.1838
Chrome 49 / 50 / Windows 7
Opera 36 / 37 / Windows 7
Does not occur on Chrome 51+
Does not occur on Opera 38+
How to reproduce
- Visit https://adblockplus.org
- Add the filter to ABP: adblockplus.org#?#:-abp-properties(width: 76px)
- Refresh the page.
Observed behaviour
Filter is not applied (ABP logo is not hidden) in the versions noted above.
Expected behaviour
Filters to work on min supported versions or min supported version number to be raised?
Attachments (0)
Change History (23)
comment:2 Changed on 09/25/2017 at 02:14:59 PM by hfiguiere
comment:3 Changed on 09/25/2017 at 02:15:23 PM by hfiguiere
- Cc hfiguiere added
comment:4 Changed on 09/25/2017 at 02:33:32 PM by Ross
No errors in the console.
comment:5 Changed on 09/25/2017 at 05:57:54 PM by hfiguiere
- Owner set to hfiguiere
comment:6 Changed on 09/25/2017 at 06:01:11 PM by hfiguiere
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:7 Changed on 09/25/2017 at 08:52:53 PM by abpbot
A commit referencing this issue has landed:
Issue 5773 - Older Opera and Chrome don't have an iterable CSSRuleList
comment:8 Changed on 09/25/2017 at 08:54:23 PM by hfiguiere
- Resolution set to fixed
- Status changed from reviewing to closed
comment:9 Changed on 09/25/2017 at 09:05:09 PM by sebastian
- Summary changed from Element hiding filters do not work in min supported Chrome/Opera versions. to Element hiding emulation filters do not work in old Chrome/Opera versions
comment:10 follow-up: ↓ 13 Changed on 09/26/2017 at 09:46:33 AM by kzar
- Cc kzar trev added
Wouldn't it have made more sense to add another workaround to give CSSRuleList iterator support adblockpluschrome/ext/common.js like we did for HTMLCollection and NodeList?
Also could you copy me in on adblockpluscore issues + reviews? This hasn't even been triaged yet but the change already got pushed :/.
comment:11 Changed on 09/26/2017 at 11:23:34 AM by trev
Seeing that I'm running Chrome 61 already, I'd rather just drop support for Chrome versions before 51. This should make this work-around and the other ones unnecessary.
comment:12 follow-up: ↓ 14 Changed on 09/26/2017 at 11:42:46 AM by kzar
Sounds good to me, according to Wikipedia version 51 is already over a year old.
comment:13 in reply to: ↑ 10 Changed on 09/26/2017 at 12:20:40 PM by hfiguiere
Replying to kzar:
Wouldn't it have made more sense to add another workaround to give CSSRuleList iterator support adblockpluschrome/ext/common.js like we did for HTMLCollection and NodeList?
Also could you copy me in on adblockpluscore issues + reviews? This hasn't even been triaged yet but the change already got pushed :/.
See issue #5381
I was unaware of a workaround in a different module for these.
comment:14 in reply to: ↑ 12 Changed on 09/26/2017 at 12:22:01 PM by hfiguiere
Replying to kzar:
Sounds good to me, according to Wikipedia version 51 is already over a year old.
Given how impossible Google made to download these old versions, I'd be in favor too.
comment:15 Changed on 09/26/2017 at 03:33:28 PM by kzar
- Resolution fixed deleted
- Status changed from closed to reopened
comment:16 Changed on 09/26/2017 at 03:35:24 PM by kzar
- Resolution set to rejected
- Status changed from reopened to closed
Marking this as rejected since we're instead going to drop support for older versions of Chrome #5795. Hubert please could you revert your earlier fix?
comment:17 Changed on 09/26/2017 at 05:06:30 PM by kzar
- Resolution rejected deleted
- Status changed from closed to reopened
Reopening for now since Sebastian questioned whether we should drop older version of Chrome after all. I still think that commit should be reverted though, since IMO it would make more sense to add the workaround to adblockpluschrome/ext/common.js if it's needed.
comment:18 Changed on 09/26/2017 at 06:09:28 PM by sebastian
comment:19 Changed on 09/27/2017 at 10:59:42 PM by hfiguiere
- Review URL(s) modified (diff)
- Status changed from reopened to reviewing
comment:20 Changed on 09/27/2017 at 11:01:41 PM by hfiguiere
- Review URL(s) modified (diff)
comment:21 Changed on 09/28/2017 at 11:58:09 AM by abpbot
A commit referencing this issue has landed:
Issue 5773 - use ES6 for stylesheets and cssRules.
comment:22 Changed on 09/28/2017 at 05:14:03 PM by abpbot
A commit referencing this issue has landed:
Issue 5773 - Polyfill iterator for CSSRuleList and StyleSheetList
comment:23 Changed on 09/28/2017 at 05:14:56 PM by sebastian
- Milestone set to Adblock-Plus-for-Chrome-Opera-next
- Resolution set to fixed
- Status changed from reviewing to closed
Any error in the console?