Sync promo: Only show skip button on startup

Sync promo: Only show skip button on startup

BUG=117456
TEST=


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

[Dan Beam, 2012-03-26 21:35:20]

https://chromiumcodereview.appspot.com/9836093/diff/1/chrome/browser/resource...
File chrome/browser/resources/sync_promo/sync_promo.js (right):

https://chromiumcodereview.appspot.com/9836093/diff/1/chrome/browser/resource...
chrome/browser/resources/sync_promo/sync_promo.js:6: <include
src="../options/options_page.js"/>
P.S. This will be removed soon, I hope options2/options_page.js works for
this...

https://chromiumcodereview.appspot.com/9836093/diff/1/chrome/browser/resource...
chrome/browser/resources/sync_promo/sync_promo.js:140:
document.documentElement.setAttribute('promo-not-startup', true);
nit: I think other attributes in NTP have no hyphens or capitialization (see:
bookmarksbarattached or hasattribution or spellcheck).

[sail, 2012-03-26 21:37:14]

https://chromiumcodereview.appspot.com/9836093/diff/1/chrome/browser/resource...
File chrome/browser/resources/sync_promo/sync_promo.js (right):

https://chromiumcodereview.appspot.com/9836093/diff/1/chrome/browser/resource...
chrome/browser/resources/sync_promo/sync_promo.js:140:
document.documentElement.setAttribute('promo-not-startup', true);
On 2012/03/26 21:35:20, Dan Beam wrote:
> nit: I think other attributes in NTP have no hyphens or capitialization (see:
> bookmarksbarattached or hasattribution or spellcheck).

Done.

[Dan Beam, 2012-03-26 22:09:00]

https://chromiumcodereview.appspot.com/9836093/diff/5001/chrome/browser/resou...
File chrome/browser/resources/sync_promo/sync_promo.js (right):

https://chromiumcodereview.appspot.com/9836093/diff/5001/chrome/browser/resou...
chrome/browser/resources/sync_promo/sync_promo.js:7: <include
src="../sync_setup_overlay.js"/>
can you put

<script src="chrome://resources/js/util.js"></script>

in sync_promo.html above the include of this script so parseQueryParams() isn't
included transitively?

https://chromiumcodereview.appspot.com/9836093/diff/5001/chrome/browser/resou...
chrome/browser/resources/sync_promo/sync_promo.js:140:
document.documentElement.setAttribute('promoNotStartup', true);
nit: I think we tend to use all lower case because it's pretty unclear in the
CSS 2.1 spec whether case matters
(http://www.w3.org/TR/CSS2/selector.html#matching-attrs), but this is more
readable, so up to you.

[sail, 2012-03-26 22:34:44]

https://chromiumcodereview.appspot.com/9836093/diff/5001/chrome/browser/resou...
File chrome/browser/resources/sync_promo/sync_promo.js (right):

https://chromiumcodereview.appspot.com/9836093/diff/5001/chrome/browser/resou...
chrome/browser/resources/sync_promo/sync_promo.js:7: <include
src="../sync_setup_overlay.js"/>
On 2012/03/26 22:09:00, Dan Beam wrote:
> can you put
> 
> <script src="chrome://resources/js/util.js"></script>
> 
> in sync_promo.html above the include of this script so parseQueryParams()
isn't
> included transitively?

Done.

https://chromiumcodereview.appspot.com/9836093/diff/5001/chrome/browser/resou...
chrome/browser/resources/sync_promo/sync_promo.js:140:
document.documentElement.setAttribute('promoNotStartup', true);
On 2012/03/26 22:09:00, Dan Beam wrote:
> nit: I think we tend to use all lower case because it's pretty unclear in the
> CSS 2.1 spec whether case matters
> (http://www.w3.org/TR/CSS2/selector.html#matching-attrs), but this is more
> readable, so up to you.

Done.

[Dan Beam, 2012-03-26 22:54:24]

lgtm

[Dan Beam, 2012-03-26 23:23:38]

https://chromiumcodereview.appspot.com/9836093/diff/2002/chrome/browser/resou...
File chrome/browser/resources/sync_promo/sync_promo.js (right):

https://chromiumcodereview.appspot.com/9836093/diff/2002/chrome/browser/resou...
chrome/browser/resources/sync_promo/sync_promo.js:141:
document.documentElement.setAttribute('isstartup', true);
also, cr.enablePlatformSpecificCSSRules() uses

  setAttribute('blah', ''); // empty string instead of true, so people
                            // can only use [attrname] not [attrname=true]

which I think is a good idea.

[sail, 2012-03-27 20:23:09]

https://chromiumcodereview.appspot.com/9836093/diff/2002/chrome/browser/resou...
File chrome/browser/resources/sync_promo/sync_promo.js (right):

https://chromiumcodereview.appspot.com/9836093/diff/2002/chrome/browser/resou...
chrome/browser/resources/sync_promo/sync_promo.js:141:
document.documentElement.setAttribute('isstartup', true);
On 2012/03/26 23:23:39, Dan Beam wrote:
> also, cr.enablePlatformSpecificCSSRules() uses
> 
>   setAttribute('blah', ''); // empty string instead of true, so people
>                             // can only use [attrname] not [attrname=true]
> 
> which I think is a good idea.

Done.

[commit-bot: I haz the power, 2012-03-27 20:23:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/9836093/8003

[commit-bot: I haz the power, 2012-03-27 23:00:07]

Try job failure for 9836093-8003 (retry) on win_rel for step "update".
It's a second try, previously, steps "ui_tests, browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-03-28 17:36:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/9836093/8003

[commit-bot: I haz the power, 2012-03-28 19:37:03]

Change committed as 129462