Issue 1755 - Create overlays with correct positioning and z-index, considering all ancestors

Issue 1755 - Create overlays with correct positioning and z-index, considering all ancestors

[kzar, 2015-01-13 11:13:13]

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
File include.postload.js (right):

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
include.postload.js:276: zIndex = Math.max(zIndex, parseInt(style.zIndex, 10));
zIndex is initially null which when passed to Math.max seems to behave as zero.
Does it matter / is it possible that if the highest zIndex in reality is a
negative number the result will be a zIndex of zero?

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
include.postload.js:289: overlay.style.zIndex = zIndex || "auto";
Should possibly check that zIndex is null here instead of just falsey?

[Sebastian Noack, 2015-01-13 13:20:09]

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
File include.postload.js (right):

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
include.postload.js:276: zIndex = Math.max(zIndex, parseInt(style.zIndex, 10));
On 2015/01/13 11:13:13, kzar wrote:
> zIndex is initially null which when passed to Math.max seems to behave as
zero.
> Does it matter / is it possible that if the highest zIndex in reality is a
> negative number the result will be a zIndex of zero?

I didn't realize that null is converted to 0, but were assuming that similar to
None in Python, it would be considered smaller than any numeric value. Fixed
that.

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
include.postload.js:289: overlay.style.zIndex = zIndex || "auto";
On 2015/01/13 11:13:13, kzar wrote:
> Should possibly check that zIndex is null here instead of just falsey?

I use "auto" as default value above now.

[kzar, 2015-01-13 13:24:50]

Assuming those extra few lines were deleted on purpose it LGTM.

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
File include.postload.js (right):

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
include.postload.js:689: default:
Did you remove these lines on purpose?

[Sebastian Noack, 2015-01-13 13:28:19]

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
File include.postload.js (right):

http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
include.postload.js:689: default:
On 2015/01/13 13:24:50, kzar wrote:
> Did you remove these lines on purpose?

I had to rebase. And those lines don't exist anymore upstream.

[kzar, 2015-01-13 13:28:56]

Ahh OK, LGTM then :)

On 2015/01/13 13:28:19, Sebastian Noack wrote:
>
http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
> File include.postload.js (right):
> 
>
http://codereview.adblockplus.org/5852223410012160/diff/5741031244955648/incl...
> include.postload.js:689: default:
> On 2015/01/13 13:24:50, kzar wrote:
> > Did you remove these lines on purpose?
> 
> I had to rebase. And those lines don't exist anymore upstream.