Opened on 11/06/2018 at 01:51:41 AM
Closed on 12/19/2018 at 02:04:41 PM
Last modified on 12/19/2018 at 02:07:20 PM
#7104 closed defect (fixed)
Element collapsing stylesheets can linger after frame is destroyed
| Reported by: | Ross | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-3.5-for-Chrome-Opera-Firefox |
| Module: | Platform | Keywords: | |
| Cc: | sebastian, kzar, mjethani | Blocked By: | |
| Blocking: | Platform: | Firefox | |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by Ross)
Environment
ABP 3.4.0.2187
Firefox 62
Could not reproduce in Chrome 70.
How to reproduce
- Create a basic HTML page with two images.
- Add a filter that blocks both images.
- Refresh page, filters apply.
- Change filter added in Step 2 to just block one image.
- Refresh the page.
Observed behaviour
After Step 4 and 5, both images are still hidden in Firefox (but not in Chrome).
Closing the tab, waiting a few seconds and reopening the page seems to fix it but not if done too quickly.
Expected behaviour
Firefox to behave as Chrome does.
Example
HTML:
<!DOCTYPE html> <head> <title>Block test</title> </head> <body> <h1>Block test</h1> <img src="test1.jpg"/> <img src="test2.jpg"/> </body> </html>
Filters:
Step 2:
test*.jpg
Step 4:
test1.jpg
Attachments (1)
Change History (20)
comment:1 Changed on 11/06/2018 at 02:24:55 AM by Ross
comment:2 Changed on 11/09/2018 at 03:46:23 PM by mjethani
- Cc mjethani added; manish removed
comment:3 Changed on 11/20/2018 at 08:37:47 PM by kzar
Please could you take a look into this one Manish?
comment:4 Changed on 11/21/2018 at 05:32:57 AM by mjethani
- Owner set to mjethani
comment:5 Changed on 11/21/2018 at 05:33:17 AM by mjethani
Ross, what is the element hiding filter here? What I see in the description above is a blocking filter.
comment:6 in reply to: ↑ description Changed on 11/21/2018 at 05:35:29 AM by mjethani
Replying to Ross:
The CSS rule to hide the image looks to be injected multiple times on page refresh.
How exactly do you see this CSS rule, and what does it look like?
Changed on 11/21/2018 at 12:41:09 PM by Ross
comment:8 follow-up: ↓ 10 Changed on 11/21/2018 at 12:48:55 PM by Ross
You're right, it's a blocking filter. I feel like I gave a bad explaination, I've updated the example above. I think there might actually be two issues here (but I also might be wrong):
- Firefox still hides/blocks an image that was previously the target of the filter, but is no longer (the filter changed). So in the updated example above, test2.jpg is still being hidden/blocked even though the filter is now test1.jpg instead of test*.jpg. The ABP icon also indicates just 1 blocked item. This is fixed after closing/reopening the tab. Also "fixed" when removing the filters.
- The second issue is that, when inspecting the image element pointing at test1.jpg in the example above, the number of display:none rules applied to it is dependent on the number of times the page has been refreshed. Each refresh adds another. This isn't exactly incorrect, as the image is hidden correctly. However the extra rules on page refresh seems to indicate something weird is happening somewhere? Chrome does not have the same behaviour.
comment:9 Changed on 12/04/2018 at 01:01:47 PM by mjethani
comment:10 in reply to: ↑ 8 Changed on 12/04/2018 at 01:08:41 PM by mjethani
Replying to Ross:
I think there might actually be two issues here [...]
This looks serious.
Based on your description, they both look like symptoms of the same problem to me: the previous style sheet is somehow still there and being applied.
comment:11 Changed on 12/04/2018 at 02:03:59 PM by mjethani
First of all, I'm seeing the issue on both Chrome and Firefox.
Technical details: When a style sheet is injected to "collapse" a blocked media element (image, video, etc.), it only appends to the existing style sheet. But the style sheet is never cleared, so it keeps appending.
This is bad because if you navigate to a different page with similar elements (like elements matching the CSS selector img[src="test1.jpg"]), the elements on the new page would also get hidden, even though the media would still be played!
Ross, you could construct a worse case here where there are two pages, and a video is blocked only on the first page. For example, let's say there's a filter /foo.mp4^$domain=foo.com, which would block the URL /foo.mp4 on foo.com but not bar.com. If you go to the page on foo.com, the URL would be blocked and the video element would be collapsed. If you then navigate in the same tab to bar.com, the video would not be blocked, but the element would still be collapsed (because of the stale style sheet), so you would hear the sound but not see anything.
comment:12 Changed on 12/04/2018 at 02:05:57 PM by mjethani
A tentative fix is to add the following line at the end of the updateFrameStructure function in ext/background.js:
frame.injectedStyleSheets = null;
comment:13 Changed on 12/04/2018 at 02:34:42 PM by mjethani
- Review URL(s) modified (diff)
Added a patch for a potential fix.
comment:14 Changed on 12/07/2018 at 12:59:34 PM by kzar
- Priority changed from Unknown to P2
- Ready set
- Summary changed from Duplicate rules are applied to element on page refresh on Firefox to Element collapsing stylesheets can linger after frame is destroyed
comment:15 Changed on 12/19/2018 at 02:01:39 PM by abpbot
A commit referencing this issue has landed:
Issue 7104 - Clear style sheets when frame structure is updated
comment:16 Changed on 12/19/2018 at 02:04:41 PM by mjethani
- Resolution set to fixed
- Status changed from new to closed
comment:17 Changed on 12/19/2018 at 02:05:32 PM by mjethani
- Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
- Platform changed from Firefox to Unknown / Cross platform
comment:18 Changed on 12/19/2018 at 02:05:55 PM by mjethani
- Platform changed from Unknown / Cross platform to Firefox
comment:19 Changed on 12/19/2018 at 02:07:20 PM by mjethani
I'm surprised that you are unable to reproduce this on Chrome, because I was able to reproduce it. Maybe it's the version (I'm on Chrome 72).
I noticed this while writing a new genericblock test page.
The page has two images that were blocked with these two filters:
/testcasefiles/genericblock/image.jpg
/testcasefiles/genericblock/image2.jpg$domain=localhost
Which ABP reports as 2 blocked items in the popup icon and both images are correctly blocked.
Adding the following filter and refreshing:
@@localhost^$genericblock
Causes ABP to report 1 on the popup icon and show the genericblock filter in the devtools panel. However both images are still blocked (or set to display: none).
Then, closing the tab and reopening the page in a new tab causes the second image to appear as expected (however this does not always work)