JavaScript style fixups in feedback.js

JavaScript style fixups in feedback.js

BUG=122002
TEST=presubmit

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=134586

[Tyler Breisacher (Chromium), 2012-04-06 18:31:38]

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
File chrome/browser/resources/feedback.js (left):

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
chrome/browser/resources/feedback.js:145: return true;
This return value is never checked, as far as I can tell.

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
chrome/browser/resources/feedback.js:164: return true;
This return value is never checked, as far as I can tell.

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
File chrome/browser/resources/feedback.js (right):

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
chrome/browser/resources/feedback.js:137: return true;
I'm not entirely sure whether this 'return true;' is necessary.

[rkc, 2012-04-06 18:34:49]

LGTM
You'll need owners approval though.

On 2012/04/06 18:31:38, Tyler Breisacher wrote:
>
http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
> File chrome/browser/resources/feedback.js (left):
> 
>
http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
> chrome/browser/resources/feedback.js:145: return true;
> This return value is never checked, as far as I can tell.
> 
>
http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
> chrome/browser/resources/feedback.js:164: return true;
> This return value is never checked, as far as I can tell.
> 
>
http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
> File chrome/browser/resources/feedback.js (right):
> 
>
http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
> chrome/browser/resources/feedback.js:137: return true;
> I'm not entirely sure whether this 'return true;' is necessary.

[Tyler Breisacher (Chromium), 2012-04-06 18:38:57]

Thanks! Moving dbeam from CC to reviewer.

[Dan Beam, 2012-04-06 20:53:57]

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
File chrome/browser/resources/feedback.js (right):

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
chrome/browser/resources/feedback.js:23: * @param {string} divId The id of the
div to search in.
The ID in which searching will occur.

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
chrome/browser/resources/feedback.js:24: * @param {string} thumbnailId The id of
the thumbnail to search for.
The ID of the thumbnail for which to search.

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
chrome/browser/resources/feedback.js:25: */
I'm just messing with you.

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
chrome/browser/resources/feedback.js:96: * @return {boolean} true if the report
was sent.
capitalize

http://codereview.chromium.org/10007041/diff/1/chrome/browser/resources/feedb...
chrome/browser/resources/feedback.js:137: return true;
On 2012/04/06 18:31:38, Tyler Breisacher wrote:
> I'm not entirely sure whether this 'return true;' is necessary.

It's probably not, as that doesn't modify the event like return false does...

[Dan Beam, 2012-04-19 02:07:50]

ping

[Tyler Breisacher (Chromium), 2012-04-19 19:47:27]

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

https://chromiumcodereview.appspot.com/10007041/diff/1/chrome/browser/resourc...
chrome/browser/resources/feedback.js:96: * @return {boolean} true if the report
was sent.
On 2012/04/06 20:53:57, Dan Beam wrote:
> capitalize

Done.

https://chromiumcodereview.appspot.com/10007041/diff/1/chrome/browser/resourc...
chrome/browser/resources/feedback.js:137: return true;
On 2012/04/06 20:53:57, Dan Beam wrote:
> On 2012/04/06 18:31:38, Tyler Breisacher wrote:
> > I'm not entirely sure whether this 'return true;' is necessary.
> 
> It's probably not, as that doesn't modify the event like return false does...

Let's leave it for now, just in case.

[Dan Beam, 2012-04-19 20:25:58]

lgtm

[Dan Beam, 2012-04-27 03:28:45]

https://chromiumcodereview.appspot.com/10007041/diff/6001/chrome/browser/reso...
File chrome/browser/resources/feedback.js (right):

https://chromiumcodereview.appspot.com/10007041/diff/6001/chrome/browser/reso...
chrome/browser/resources/feedback.js:133: * @return {boolean} true
remove this and change to take |e| param and call e.preventDefault() IMO

[rkc, 2012-04-30 20:15:14]

This was created 3 weeks ago, LGTM'ed a week ago; any reason why we're waiting
on committing it?

On 2012/04/27 03:28:45, Dan Beam wrote:
>
https://chromiumcodereview.appspot.com/10007041/diff/6001/chrome/browser/reso...
> File chrome/browser/resources/feedback.js (right):
> 
>
https://chromiumcodereview.appspot.com/10007041/diff/6001/chrome/browser/reso...
> chrome/browser/resources/feedback.js:133: * @return {boolean} true
> remove this and change to take |e| param and call e.preventDefault() IMO

[Tyler Breisacher (Chromium), 2012-04-30 20:16:38]

Sorry, my bad. Will commit now :)