Opened on 04/17/2014 at 02:12:05 PM
Closed on 01/21/2015 at 06:31:52 AM
Last modified on 02/20/2015 at 05:00:32 PM
#350 closed defect (fixed)
"Block element" dialog isn’t completely visible when selecting elements inside small frames
| Reported by: | philll | Assignee: | kzar |
|---|---|---|---|
| Priority: | P3 | Milestone: | Adblock-Plus-1.8.11-for-Chrome-Opera-Safari |
| Module: | Platform | Keywords: | |
| Cc: | smultron45@gmail.com, sebastian, trev, manvel@adblockplus.org | Blocked By: | |
| Blocking: | #1444 | Platform: | Chrome |
| Ready: | yes | Confidential: | no |
| Tester: | Verified working: | no | |
| Review URL(s): | |||
Description (last modified by trev)
Environment
Chrome 34/35
ABP 1.7.4 with no filters active
How to reproduce
- Go to doregama.info
- Right click on the ad at the extreme right bottom and select ‘Block Element’ from the right click menu
Observed behaviour
The "add Filter(s)" dialog is not completely visible as displayed in attachment:challengebug_5706.png. Works fine if using the "block element" option from the ABP icon menu.
Taken from http://99tests.com/challenges/56/challengebugs/5706
Expected behaviour
The "add Filter(s)" dialog should be completely visible.
Attachments (1)
Change History (19)
comment:2 Changed on 04/17/2014 at 03:21:35 PM by mapx
- Cc smultron45@gmail.com added
comment:3 Changed on 04/17/2014 at 06:19:14 PM by sebastian
comment:4 Changed on 04/30/2014 at 07:47:22 AM by philll
- Ready set
Changed on 05/23/2014 at 04:03:00 PM by philll
comment:6 Changed on 06/24/2014 at 10:27:24 AM by sebastian
- Cc sebastian trev added
Alternatively, we could also disable the "Block element" feature in frames that are too small to show the dialog. This would make things way simpler. You could still select the entire frame to block, which might be the preferred behavior in many cases anyway. What do you think, trev and mapx?
comment:7 Changed on 06/24/2014 at 10:44:12 AM by mapx
is the first solution you "saw" ("inject the overlay into the top-level frame") too complicated to implement ? top-level frame would be the page itself (where the dialogue window has enough space to be showed), right ?
comment:8 Changed on 06/24/2014 at 11:50:40 AM by trev
- Description modified (diff)
Injecting the dialog into the top frame would certainly be better. I see the following issues:
- Translating coordinates between frames: window.screenX seems to be supported by both Chrome and Safari, this should give us the position of the frames relative to each other.
- Communicating with the top frame: we can send a message to all frames and only the frame with parent == window will act upon it.
- We might not have a content script running in the top frame: we could still fall back to the frame containing the element or just ignore this scenario as it is fairly uncommon.
Other than that, the complication seems to be mostly that additional messaging will be introduced. There should be no significant differences between Chrome and Safari here either. Sebastian, any additional issues that you see here?
comment:9 Changed on 06/24/2014 at 12:13:40 PM by sebastian
I don't know of any issues that would prevent us from injecting the dialog always in the top-level frame. However we would have to add a fair amount of complexity, to decouple the code that listens for mouse events, inspects the DOM and finally removes the element and the code rendering and processing the dialog.
I am not sure if that is worth it, considering that this is rather a corner case, in which it seems to be even preferable to block the entire frame. I played a little around with the code and it seems that we could just one check in order to ignore the mouse events in small frames, that if you hover/click inside that frame the parent frame will get the event and you can block the entire frame.
comment:10 Changed on 07/05/2014 at 03:25:03 AM by saroyanm
- Cc manvel@adblockplus.org added
- Platform set to Unknown
comment:11 Changed on 07/15/2014 at 01:55:16 PM by trev
- Component changed from Unknown to Platform
- Platform changed from Unknown to Chrome
comment:12 Changed on 08/25/2014 at 09:58:00 AM by trev
- Priority changed from P2 to P3
comment:13 Changed on 09/25/2014 at 01:24:56 PM by sebastian
Also see #1443.
comment:14 Changed on 09/25/2014 at 02:21:30 PM by greiner
- Blocking 1444 added
comment:15 Changed on 01/13/2015 at 01:21:31 PM by kzar
- Owner set to kzar
comment:16 Changed on 01/16/2015 at 03:26:00 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:17 Changed on 01/21/2015 at 06:31:52 AM by sebastian
- Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:18 Changed on 02/20/2015 at 05:00:32 PM by sebastian
- Summary changed from Add Filter(s) dialog is not completely visible for some ads to "Block element" dialog isn’t completely visible when selecting elements inside small frames
The problem here is that the overlay is injected inside an iframe, which is smaller than the overlay. In order to fix this we must always inject the overlay into the top-level frame.