Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#1082 closed change (fixed)

Use Element.getBoundingClientRect to position overlay in "block element" feature

Reported by: greiner Assignee: sebastian
Priority: P3 Milestone: Adblock-Plus-1.8.11-for-Chrome-Opera-Safari
Module: Platform Keywords: goodfirstbug
Cc: mapx Blocked By:
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/6527157522137088

Description (last modified by sebastian)

Background

We currently determine the position of an element on a page ourselves by adding up offsets to its parent elements. This is required for the "Block element" feature. However, this functionality is already provided by the browser by getBoundingClientRect(). Moreover, our implementation doesn't work for framesets.

What to change

In include.postload.js:

  • Remove getAbsolutePosition function
  • Use Element.getBoundingClientRect in conjunction with window.scrollX and window.scrollY instead

Hints for testers

Verify that when using "Block element" the highlighted boxes are positioned as before this change. That means that the yellow boxes should be located on top of the elements underneath the mouse cursor and that the red boxes should be located on top of the elements matching the suggested filter.

Test on Chromium based browsers and Safari.

Change History (9)

comment:1 Changed 5 years ago by mapx

  • Cc mapx added

comment:2 Changed 5 years ago by sebastian

How about adding some information to this issue, like why this approach is preferable, and what the expected results are, and how to test it?

comment:3 Changed 5 years ago by sebastian

Last edited 5 years ago by sebastian (previous) (diff)

comment:4 Changed 5 years ago by greiner

  • Description modified (diff)

Added "Hints for testers" section.

comment:5 Changed 5 years ago by sebastian

  • Description modified (diff)
  • Platform changed from Chrome to Unknown
  • Priority changed from P3 to P4
  • Ready set

comment:6 Changed 5 years ago by sebastian

  • Owner set to sebastian
  • Priority changed from P4 to P3

Increasing priority, since the current approach beside being obsolete, causes issues with framesets.

comment:7 Changed 5 years ago by sebastian

  • Description modified (diff)
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:8 Changed 5 years ago by sebastian

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:9 Changed 5 years ago by sebastian

  • Description modified (diff)
Note: See TracTickets for help on using tickets.