Opened on 09/12/2018 at 10:03:10 PM
Closed on 10/01/2018 at 02:13:07 PM
Last modified on 02/18/2019 at 11:55:59 AM
#6948 closed defect (fixed)
"block element" bug in ABP 3.3.2 (only chrome)
| Reported by: | mapx | Assignee: | kzar |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-3.5-for-Chrome-Opera-Firefox |
| Module: | Platform | Keywords: | |
| Cc: | sebastian, mjethani, kzar, greiner, Ross | Blocked By: | |
| Blocking: | Platform: | Chrome | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by kzar)
Environment
windows 10
chrome 69
ABP 3.3.2
How to reproduce
- go to a page with images, for example here:
https://www.purelypoultry.com/pekin-ducklings-p-621.html
- right click an image, block element, confirm, the image is blocked
- right click another image on the same page
Observed behaviour
The "block element" option / feature disappeared
Note
- It's ok in ABP for firefox
- to see again the "block element" option just click another tab and then go back to the images page
Reported here:
https://adblockplus.org/forum/viewtopic.php?f=10&t=59376&start=0
Hints for testers
With this fix I rewrote the context menu logic, so we'll need to test our "Block element" right-click context menu button quite thoroughly.
On Firefox for Android the context menu should not be displayed, since the filter composer isn't designed for mobile and some of the required APIs aren't available. Please test that general adblocking functionality works OK there, since this change has modified the "If not Firefox for Android" checks.
On other platforms, the "Block element" context menu is supposed to show up when the user right-clicks on image, video and audio elements. But only if the page isn't whitelisted, and if the page was loaded after the extension was last installed/reloaded. When the "Block element" button in the context menu is clicked, the filter composer window should pop up and the element which was right clicked should be highlighted red.
Behind the scenes, the browser actually only lets us add or remove our context menu item for _all_ tabs in all windows at once. So, we actually watch to see when the user changes tab, to check if we should display the context menu for the current tab. Supposing the user switches from one whitelisted tab to another, we don't have to do anything, but then if they switch to a non-whitelisted page we might have to then add our context menu item.
So, test things like switching between tabs and windows and check that the context menu is displayed when it should be.
Finally, some logic relating to if the "Block Element" button in the popup was touched with this change. So, test that still works too.
Attachments (0)
Change History (12)
comment:3 Changed on 09/12/2018 at 10:43:46 PM by sebastian
- Priority changed from Unknown to P2
- Ready set
comment:4 Changed on 09/13/2018 at 02:55:04 PM by mapx
ABP console
Unchecked runtime.lastError while running browserAction.setIcon: No tab with id: 817543195.
comment:5 Changed on 09/13/2018 at 02:58:31 PM by sebastian
This issue has been reported before, see #6490. But it might be unrelated of the behavior reported here, as this error already showed up before Chrome 69.
comment:6 Changed on 09/24/2018 at 02:44:36 PM by kzar
- Owner set to kzar
I can reproduce this on Chrome 68, investigating.
comment:7 Changed on 09/28/2018 at 03:40:32 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:10 Changed on 10/01/2018 at 02:11:40 PM by abpbot
A commit referencing this issue has landed:
Issue 6948 - Begun simplifying the "block element" context menu logic
comment:11 Changed on 10/01/2018 at 02:13:07 PM by kzar
- Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:12 Changed on 02/18/2019 at 11:55:59 AM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Fixed. The context menu entry does not disappear after first use, is hidden when the page is whitelisted and is kept in sync with changing tab/whitelist state.
This still does occur on Edge however, so I've opened #7299 separately.
ABP 3.4.3.2253
Firefox 65.0.1 / 51 / Windows 10
Firefox Mobile 65.0.1 / Android 7.1.1
Chrome 72.0.3626.109 / 49.0.2623.75 / Windows 10
Opera 58.0.3135.65 / 36.0.2130.80 / Windows 10
Edge 44.17763.1.0 / Windows 10
I could reproduce it on Chrome 69 with Adblock Plus 3.2, 3.3.2 and on the "next" branch as of revision 811116e9757e, but sometimes it only happened after using "Block element" twice. I couldn't reproduce it at all on Chrome 68.