Update firefox addon welcome page (issue1150001)

[ls...@google.com]

Reviewers: bmcquade,



Please review this at http://page-speed-codereview.appspot.com/1150001/

Affected files:
M build/build_xpi.gyp
M pagespeed_firefox/cpp/pagespeed/pagespeed_rules.cc
A pagespeed_firefox/js/pagespeed/pagespeed-panel.css
M pagespeed_firefox/js/pagespeed/pagespeedLibraryRules.js
M pagespeed_firefox/js/pagespeed/pagespeedPanel.js
A
pagespeed_firefox/xpi_resources/chrome/pagespeed/content/pagespeed-128.png
M
pagespeed_firefox/xpi_resources/chrome/pagespeed/content/pagespeed-32.png
M
pagespeed_firefox/xpi_resources/chrome/pagespeed/content/pagespeed-64.png
M pagespeed_firefox/xpi_resources/defaults/preferences/pagespeed.js
M pagespeed_firefox/xpi_resources/icon.png

[bmcq...@google.com]

Thanks for these changes! Looks good. A couple questions.


http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/cpp/pagespeed/pagespeed_rules.cc
File pagespeed_firefox/cpp/pagespeed/pagespeed_rules.cc (left):

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/cpp/pagespeed/pagespeed_rules.cc#oldcode445
pagespeed_firefox/cpp/pagespeed/pagespeed_rules.cc:445:
rule_results->set_rule_score(100);
since we arent rebuilding the binaries i'm disinclined to make any
native code changes in this CL.

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedLibraryRules.js
File pagespeed_firefox/js/pagespeed/pagespeedLibraryRules.js (right):

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedLibraryRules.js#newcode56
pagespeed_firefox/js/pagespeed/pagespeedLibraryRules.js:56:
strings.push(' See <a href="', url, '" target=_blank>',
why this change?

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedPanel.js
File pagespeed_firefox/js/pagespeed/pagespeedPanel.js (right):

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedPanel.js#newcode194
pagespeed_firefox/js/pagespeed/pagespeedPanel.js:194: A({'href':
'https://developers.google.com/speed/pagespeed/insights/',
odd wrapping. is there a newline present or is this a bug with our code
review tool?

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedPanel.js#newcode214
pagespeed_firefox/js/pagespeed/pagespeedPanel.js:214: ' for detailed
information on the rules used to evaluate web pages.'
this wrapping is odd. is there a newline in there?

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedPanel.js#newcode959
pagespeed_firefox/js/pagespeed/pagespeedPanel.js:959:
Firebug.registerStylesheet("chrome://pagespeed/content/pagespeed-panel.css");
do you know if doing this applies to other panels as well? or just our
panel?

http://page-speed-codereview.appspot.com/1150001/

[ls...@google.com]

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/cpp/pagespeed/pagespeed_rules.cc
File pagespeed_firefox/cpp/pagespeed/pagespeed_rules.cc (left):

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/cpp/pagespeed/pagespeed_rules.cc#oldcode445
pagespeed_firefox/cpp/pagespeed/pagespeed_rules.cc:445:
rule_results->set_rule_score(100);

On 2014/04/11 14:03:52, bmcquade wrote:
> since we arent rebuilding the binaries i'm disinclined to make any
native code
> changes in this CL.

Without this change, we are not able to run the make. I can revert this
after I build the XPI -- with old binaries.

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedLibraryRules.js
File pagespeed_firefox/js/pagespeed/pagespeedLibraryRules.js (right):

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedLibraryRules.js#newcode56
pagespeed_firefox/js/pagespeed/pagespeedLibraryRules.js:56:
strings.push(' See <a href="', url, '" target=_blank>',

On 2014/04/11 14:03:52, bmcquade wrote:
> why this change?
Fix issue: https://code.google.com/p/page-speed/issues/detail?id=1650

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedPanel.js
File pagespeed_firefox/js/pagespeed/pagespeedPanel.js (right):

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedPanel.js#newcode194
pagespeed_firefox/js/pagespeed/pagespeedPanel.js:194: A({'href':
'https://developers.google.com/speed/pagespeed/insights/',

On 2014/04/11 14:03:52, bmcquade wrote:
> odd wrapping. is there a newline present or is this a bug with our
code review
> tool?

It is the wrap of the code review.

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedPanel.js#newcode214
pagespeed_firefox/js/pagespeed/pagespeedPanel.js:214: ' for detailed
information on the rules used to evaluate web pages.'

On 2014/04/11 14:03:52, bmcquade wrote:
> this wrapping is odd. is there a newline in there?

no newline.

http://page-speed-codereview.appspot.com/1150001/diff/20002/pagespeed_firefox/js/pagespeed/pagespeedPanel.js#newcode959
pagespeed_firefox/js/pagespeed/pagespeedPanel.js:959:
Firebug.registerStylesheet("chrome://pagespeed/content/pagespeed-panel.css");

On 2014/04/11 14:03:52, bmcquade wrote:
> do you know if doing this applies to other panels as well? or just our
panel?

No. I do not know how this css works, but it works for us. I do not
notice any abnormality in other panels.

http://page-speed-codereview.appspot.com/1150001/

[bmcq...@google.com]

LGTM

http://page-speed-codereview.appspot.com/1150001/