Issue 2173 - Fixed: "Block element" dialog isn't moved to correct position when dragging

Issue 2173 - Fixed: "Block element" dialog isn't moved to correct position when dragging

[kzar, 2015-03-19 16:15:15]

Seems a little ugly but I can't see how else to do it, LGTM

http://codereview.adblockplus.org/5766275812818944/diff/5741031244955648/bloc...
File block.js (right):

http://codereview.adblockplus.org/5766275812818944/diff/5741031244955648/bloc...
block.js:97: var dragStartX;
Nit: probably should set the start coords to null as well to keep it consistent.

[Sebastian Noack, 2015-03-19 16:52:14]

http://codereview.adblockplus.org/5766275812818944/diff/5741031244955648/bloc...
File block.js (right):

http://codereview.adblockplus.org/5766275812818944/diff/5741031244955648/bloc...
block.js:97: var dragStartX;
On 2015/03/19 16:15:15, kzar wrote:
> Nit: probably should set the start coords to null as well to keep it
consistent.

Note that we explicitly check for null when accessing dragEndX and dragEndY
because we might do so before they were set.

However, dragStartX and dragStarY should be set before we access it, otherwise
there is a bug, and it's intended that strict-mode throws a warning then.

[kzar, 2015-03-19 16:55:01]

http://codereview.adblockplus.org/5766275812818944/diff/5741031244955648/bloc...
File block.js (right):

http://codereview.adblockplus.org/5766275812818944/diff/5741031244955648/bloc...
block.js:97: var dragStartX;
On 2015/03/19 16:52:14, Sebastian Noack wrote:
> On 2015/03/19 16:15:15, kzar wrote:
> > Nit: probably should set the start coords to null as well to keep it
> consistent.
> 
> Note that we explicitly check for null when accessing dragEndX and dragEndY
> because we might do so before they were set.
> 
> However, dragStartX and dragStarY should be set before we access it, otherwise
> there is a bug, and it's intended that strict-mode throws a warning then.

Wow, it never occurred to me that `10 - null` would not throw an exception...
but I just tried it and it doesn't. Fair enough then.

[Sebastian Noack, 2015-03-19 17:06:44]

On 2015/03/19 16:55:01, kzar wrote:
>
http://codereview.adblockplus.org/5766275812818944/diff/5741031244955648/bloc...
> File block.js (right):
> 
>
http://codereview.adblockplus.org/5766275812818944/diff/5741031244955648/bloc...
> block.js:97: var dragStartX;
> On 2015/03/19 16:52:14, Sebastian Noack wrote:
> > On 2015/03/19 16:15:15, kzar wrote:
> > > Nit: probably should set the start coords to null as well to keep it
> > consistent.
> > 
> > Note that we explicitly check for null when accessing dragEndX and dragEndY
> > because we might do so before they were set.
> > 
> > However, dragStartX and dragStarY should be set before we access it,
otherwise
> > there is a bug, and it's intended that strict-mode throws a warning then.
> 
> Wow, it never occurred to me that `10 - null` would not throw an exception...
> but I just tried it and it doesn't. Fair enough then.

Well, neither does 10 - undefined. But at least in strict mode, you get a
warning when attempting to get the value of an uninitialized variable.

[Thomas Greiner, 2015-03-20 11:57:00]

LGTM

Not a fan of user agent string checks myself but in this case the alternatives
would cause too much overhead with too little extra benefit, so it's fine with
me.