Sync UI: Ensure the close button is shown.

Sync UI: Ensure the close button is shown.

Get rid of the multitude of close buttons.

BUG=124648
TEST=none
R=dbeam


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=133737

[Dan Beam, 2012-04-23 21:19:55]

overall I like removing code/making things reusable, though, so this is a good
idea

https://chromiumcodereview.appspot.com/10187008/diff/1/chrome/browser/resourc...
File chrome/browser/resources/sync_setup_overlay.js (right):

https://chromiumcodereview.appspot.com/10187008/diff/1/chrome/browser/resourc...
chrome/browser/resources/sync_setup_overlay.js:515: for (var i = 1; i <
overlay.children.length; i++)
why not

  var children = document.querySelectorAll(
      '#sync-setup-overlay > *:not(.close-button)');

or

  if (overlay.children[i].classList.contains('close-button'))
    continue;

thanks for documenting why you're starting at 1 but I'm just concerned that if
the position of close-button ever changed there'd be another subtle bug

[James Hawkins, 2012-04-24 16:16:27]

https://chromiumcodereview.appspot.com/10187008/diff/1/chrome/browser/resourc...
File chrome/browser/resources/sync_setup_overlay.js (right):

https://chromiumcodereview.appspot.com/10187008/diff/1/chrome/browser/resourc...
chrome/browser/resources/sync_setup_overlay.js:515: for (var i = 1; i <
overlay.children.length; i++)
On 2012/04/23 21:19:55, Dan Beam wrote:
> why not
> 
>   var children = document.querySelectorAll(
>       '#sync-setup-overlay > *:not(.close-button)');
> 
> or
> 
>   if (overlay.children[i].classList.contains('close-button'))
>     continue;
> 
> thanks for documenting why you're starting at 1 but I'm just concerned that if
> the position of close-button ever changed there'd be another subtle bug

Done.

[James Hawkins, 2012-04-24 16:54:47]

+csilv

[csilv, 2012-04-24 16:59:31]

lgtm

[commit-bot: I haz the power, 2012-04-24 17:02:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jhawkins@chromium.org/10187008/5001

[Dan Beam, 2012-04-24 17:09:24]

lgtm

[commit-bot: I haz the power, 2012-04-24 18:50:53]

Change committed as 133737