First attempt at creating a first-run page

The page doesn`t open automatically yet, you have to go to Options, open JavaScript Console and run location.href="firstRun.html" there. Also, there isn`t anything donation-related yet. Changelog link points nowhere - that`s because the next version no longer should have "beta" in the announcement URL.

[Wladimir Palant, 2012-09-28 20:03:16]

I updated the patch set here - now there are two variants of Share buttons, one
is being chosen randomly.

[Felix Dahlke, 2012-10-02 12:36:04]

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.json
File _locales/en/messages.json (right):

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:112: "message": "Support websites with acceptable ads
(read below for more information)"
"by allowing" instead of "with"?
"read more below" instead of "read below for more information"?

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:116: "message": "Surf the web without annoying ads"
"Enjoy" instead of "Surf"?

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:120: "message": "Many other improvements (<a>view
changelog</a>)"
As you suggested, maybe we can say "Constantly improved (view changelog)" or
something along those lines. It doesn't sound like a feature right now.

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:124: "message": "Do you like blocking ads?"
As I said yesterday, this is a very important line, we should put some thought
into it.

My ideas:

"Love the web without annoying ads?"
"Love Adblock Plus?"

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:128: "message": "Please spread the word!"
Likewise, this is important. Some more ideas by me:

"Spread the word!" (no please)
"Get in touch!"

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:144: "message": "Adblock Plus has been installed."
This sounds a bit technical. How about something like this:

"Welcome to Adblock Plus!"
"Thanks for installing Adblock Plus!"

http://codereview.adblockplus.org/8403145/diff/20001/background.js
File background.js (right):

http://codereview.adblockplus.org/8403145/diff/20001/background.js#newcode16
background.js:16: var isFirstRun = false;
It seems like this variable is never read, so I guess it's redundant.

http://codereview.adblockplus.org/8403145/diff/20001/firstRun.js
File firstRun.js (right):

http://codereview.adblockplus.org/8403145/diff/20001/firstRun.js#newcode18
firstRun.js:18: var versionID =
chrome.app.getDetails().version.split(".").slice(0, 2).join("");
Shouldn't this be named versionId, analogous to titleId above?

http://codereview.adblockplus.org/8403145/diff/20001/skin/firstRun.css
File skin/firstRun.css (right):

http://codereview.adblockplus.org/8403145/diff/20001/skin/firstRun.css#newcode13
skin/firstRun.css:13: h1
It looks a bit funny that the logo is centered but the text isn't. I've played
around a bit and I like the way it looks if the logo is actually in front of the
text.

Here's the code (replace the h1 block with it):

h1
{
  height: 128px;
  line-height: 128px;
  vertical-align: top;
  font-size: 220%;
  font-weight: normal;
}

h1:before
{
  content: url(/icons/abp-128.png);
  margin-right: 10px;
}

Note that this is just a quick hack, it will look funky when the browser window
gets so small that the headline text is wrapped.

On a related note, I think we should turn the ABP logo into a link to
http://www.adblockplus.org.

http://codereview.adblockplus.org/8403145/diff/20001/skin/firstRun.css#newcode41
skin/firstRun.css:41: :root[share-variant="1"] #share1
You could merge this with the following rule.

http://codereview.adblockplus.org/8403145/diff/20001/skin/firstRun.css#newcod...
skin/firstRun.css:106: background-image: linear-gradient(0deg, #FFFDF8, #FEDE9E
60%, #FFAF34 65%, #FFEFD3);
I don't think this is necessary when the first-run page is Chrome-only.

[Felix Dahlke, 2012-10-15 14:55:05]

I've marked most of my translation comments as done, since the whole team will
address them.

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.json
File _locales/en/messages.json (right):

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:112: "message": "Support websites with acceptable ads
(read below for more information)"
On 2012/10/02 12:36:04, Felix H. Dahlke wrote:
> "by allowing" instead of "with"?
> "read more below" instead of "read below for more information"?

Done.

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:116: "message": "Surf the web without annoying ads"
On 2012/10/02 12:36:04, Felix H. Dahlke wrote:
> "Enjoy" instead of "Surf"?

Done.

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:120: "message": "Many other improvements (<a>view
changelog</a>)"
On 2012/10/02 12:36:04, Felix H. Dahlke wrote:
> As you suggested, maybe we can say "Constantly improved (view changelog)" or
> something along those lines. It doesn't sound like a feature right now.

Done.

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:124: "message": "Do you like blocking ads?"
On 2012/10/02 12:36:04, Felix H. Dahlke wrote:
> As I said yesterday, this is a very important line, we should put some thought
> into it.
> 
> My ideas:
> 
> "Love the web without annoying ads?"
> "Love Adblock Plus?"

Done.

http://codereview.adblockplus.org/8403145/diff/20001/_locales/en/messages.jso...
_locales/en/messages.json:128: "message": "Please spread the word!"
On 2012/10/02 12:36:04, Felix H. Dahlke wrote:
> Likewise, this is important. Some more ideas by me:
> 
> "Spread the word!" (no please)
> "Get in touch!"

Done.

[Wladimir Palant, 2012-10-17 10:25:39]

http://codereview.adblockplus.org/8403145/diff/20001/background.js
File background.js (right):

http://codereview.adblockplus.org/8403145/diff/20001/background.js#newcode16
background.js:16: var isFirstRun = false;
On 2012/10/02 12:36:04, Felix H. Dahlke wrote:
> It seems like this variable is never read, so I guess it's redundant.

Yes, that's code from ABP/Chrome used by the first-run page. If we manage to get
acceptable ads working in the initial release we will need to add the first run
page.

http://codereview.adblockplus.org/8403145/diff/20001/skin/firstRun.css
File skin/firstRun.css (right):

http://codereview.adblockplus.org/8403145/diff/20001/skin/firstRun.css#newcod...
skin/firstRun.css:106: background-image: linear-gradient(0deg, #FFFDF8, #FEDE9E
60%, #FFAF34 65%, #FFEFD3);
On 2012/10/02 12:36:04, Felix H. Dahlke wrote:
> I don't think this is necessary when the first-run page is Chrome-only.

The expectation here is that Webkit will eventually implement the
standard-compliant version and drop the old one.

[Felix Dahlke, 2012-10-17 15:21:34]

LGTM!