Issue 1128 - Added ADC as supporter to Manifesto page

Issue 1128 - Added ADC as supporter to Manifesto page

[Wladimir Palant, 2014-08-13 13:13:51]

http://codereview.adblockplus.org/5119431676002304/diff/5629499534213120/body...
File body.html (right):

http://codereview.adblockplus.org/5119431676002304/diff/5629499534213120/body...
body.html:94: <span id="supporters-overflow" hidden="true">
This seems to be reverting changes you implemented earlier. Is that intentional?

[Thomas Greiner, 2014-08-13 14:23:36]

http://codereview.adblockplus.org/5119431676002304/diff/5629499534213120/body...
File body.html (right):

http://codereview.adblockplus.org/5119431676002304/diff/5629499534213120/body...
body.html:94: <span id="supporters-overflow" hidden="true">
On 2014/08/13 13:13:51, Wladimir Palant wrote:
> This seems to be reverting changes you implemented earlier. Is that
intentional?

The idea was to remove the overflow until we have enough supporters to fill two
rows and have at least two supporters in the overflow section.

Due to the late removal of the overflow section (see #628) and the unexpected
timely addition of ADC, that occured quicker than expected.

Besides that, the overflow section is now behaving differently than before. It's
no longer a separate row being shown inline. The link is also no longer meant to
toggle the overflow on and off anymore.

[Wladimir Palant, 2014-08-13 20:44:47]

http://codereview.adblockplus.org/5119431676002304/diff/5629499534213120/head...
File head.html (right):

http://codereview.adblockplus.org/5119431676002304/diff/5629499534213120/head...
head.html:520: ev.preventDefault();
Had to look up - this is only implemented starting with MSIE9. Should we make
this backwards compatible?

if ("preventDefault" in ev)
  ev.preventDefault();
else
  ev.returnValue = false; // MSIE 8.0 and below

[Thomas Greiner, 2014-08-16 08:59:30]

http://codereview.adblockplus.org/5119431676002304/diff/5629499534213120/head...
File head.html (right):

http://codereview.adblockplus.org/5119431676002304/diff/5629499534213120/head...
head.html:520: ev.preventDefault();
On 2014/08/13 20:44:47, Wladimir Palant wrote:
> Had to look up - this is only implemented starting with MSIE9. Should we make
> this backwards compatible?
> 
> if ("preventDefault" in ev)
>   ev.preventDefault();
> else
>   ev.returnValue = false; // MSIE 8.0 and below

We already consider IE8 with the addListener() function so I'm in favor of
making this backwards compatible. Apparently, preventDefault() was introduced
together with addEventListener() so I decided to handle that directly in
addListener().

[Wladimir Palant, 2014-08-18 12:31:19]

http://codereview.adblockplus.org/5119431676002304/diff/5717271485874176/head...
File head.html (right):

http://codereview.adblockplus.org/5119431676002304/diff/5717271485874176/head...
head.html:500: ev.preventDefault();
A more consistent approach would be using the return value instead of a new
parameter:

  if (listener(ev) === false)
    ev.preventDefault();

That's essentially what you get with onclick and such.

http://codereview.adblockplus.org/5119431676002304/diff/5717271485874176/head...
head.html:511: event.returnValue = preventDefault;
This should assign !preventDefault - see onclick="return false;" (return true
would allow the default handling to execute).

[Thomas Greiner, 2014-08-19 09:32:34]

http://codereview.adblockplus.org/5119431676002304/diff/5717271485874176/head...
File head.html (right):

http://codereview.adblockplus.org/5119431676002304/diff/5717271485874176/head...
head.html:500: ev.preventDefault();
On 2014/08/18 12:31:19, Wladimir Palant wrote:
> A more consistent approach would be using the return value instead of a new
> parameter:
> 
>   if (listener(ev) === false)
>     ev.preventDefault();
> 
> That's essentially what you get with onclick and such.

Done.

http://codereview.adblockplus.org/5119431676002304/diff/5717271485874176/head...
head.html:511: event.returnValue = preventDefault;
On 2014/08/18 12:31:19, Wladimir Palant wrote:
> This should assign !preventDefault - see onclick="return false;" (return true
> would allow the default handling to execute).

Done.

[Wladimir Palant, 2014-08-19 09:48:59]

LGTM