Make email field in Feedback app uneditable

Make email field in Feedback app uneditable

Change the text field to a drop down with an option to let users report
anonymously.
Also make the minimize and close buttons recognizable by ChromeVox

SCREEN_SHOT=https://screenshot.googleplex.com/5nmVg5dyUdG.png
BUG=679958
TEST=browser_tests --gtest_filter=FeedbackTest.AnonymousUser
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2728773003
Cr-Commit-Position: refs/heads/master@{#454396}
Committed: https://chromium.googlesource.com/chromium/src/+/6795dfa80c19f7e44dc58c37f594b403dab9326e

[afakhry, 2017-03-02 02:43:20]

Description was changed from

==========
Make email field in Feedback app uneditable

Change the text field to a drop down with an option to let users report
anonymously.
Also make the minimize and close buttons recognizable by ChromeVox

SCREEN_SHOT=https://screenshot.googleplex.com/5nmVg5dyUdG.png
BUG=679958
TEST=browser_tests --gtest_filter=FeedbackTest.AnonymousUser
==========

to

==========
Make email field in Feedback app uneditable

Change the text field to a drop down with an option to let users report
anonymously.
Also make the minimize and close buttons recognizable by ChromeVox

SCREEN_SHOT=https://screenshot.googleplex.com/5nmVg5dyUdG.png
BUG=679958
TEST=browser_tests --gtest_filter=FeedbackTest.AnonymousUser
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[afakhry, 2017-03-02 02:43:26]

The CQ bit was checked by afakhry@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 02:44:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[afakhry, 2017-03-02 02:45:17]

afakhry@chromium.org changed reviewers:
+ xiyuan@chromium.org

[afakhry, 2017-03-02 02:45:18]

Xiyuan, could you please review this CL? Thanks!

[commit-bot: I haz the power, 2017-03-02 03:35:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 03:35:16]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[afakhry, 2017-03-02 18:37:44]

The CQ bit was checked by afakhry@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 18:38:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xiyuan, 2017-03-02 18:55:11]

Just nits

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/feedback/css/feedback.css (right):

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
chrome/browser/resources/feedback/css/feedback.css:79: border-color: #c8c8c8;
nit: this can fold into the previous line,
i.e. border: 1px solid #c8c8c8;

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/feedback/js/feedback.js (right):

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:57: var
ANONYMOUS_USER_OPTION_VALUE = 'anonymous_user';
nit: not used?

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:175: feedbackInfo.email =
emailDropDown.options[emailDropDown.selectedIndex].value;
nit: get rid of |emailDropDown| and use $('user-email-drop-down') in place

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:359:
dropDown.insertBefore(optionElement, dropDown.firstChild);
nit: get rid of |dropDown|

[afakhry, 2017-03-02 19:04:13]

The CQ bit was checked by afakhry@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 19:04:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[afakhry, 2017-03-02 19:04:48]

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/feedback/css/feedback.css (right):

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
chrome/browser/resources/feedback/css/feedback.css:79: border-color: #c8c8c8;
On 2017/03/02 18:55:11, xiyuan wrote:
> nit: this can fold into the previous line,
> i.e. border: 1px solid #c8c8c8;

Done.

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/feedback/js/feedback.js (right):

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:57: var
ANONYMOUS_USER_OPTION_VALUE = 'anonymous_user';
On 2017/03/02 18:55:11, xiyuan wrote:
> nit: not used?

Yes, not anymore, forgot to remove it. Done.

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:175: feedbackInfo.email =
emailDropDown.options[emailDropDown.selectedIndex].value;
On 2017/03/02 18:55:11, xiyuan wrote:
> nit: get rid of |emailDropDown| and use $('user-email-drop-down') in place

The line would be too long, but done anyway.

https://codereview.chromium.org/2728773003/diff/20001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:359:
dropDown.insertBefore(optionElement, dropDown.firstChild);
On 2017/03/02 18:55:11, xiyuan wrote:
> nit: get rid of |dropDown|

Done.

[xiyuan, 2017-03-02 19:23:34]

https://codereview.chromium.org/2728773003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/feedback/js/feedback.js (right):

https://codereview.chromium.org/2728773003/diff/40001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:170:
$('user-email-drop-down').options[
feedbackInfo.email = $('user-email-drop-down').value;

https://codereview.chromium.org/2728773003/diff/40001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:355: optionElement,
$('user-email-drop-down').firstChild);
oops, did not realized that $('user-email-drop-down') is used twice. In this
case, we should use a var like you had before. :p

Or use $('anonymous-user-option').

[afakhry, 2017-03-02 19:55:55]

The CQ bit was checked by afakhry@chromium.org to run a CQ dry run

[afakhry, 2017-03-02 19:56:07]

https://codereview.chromium.org/2728773003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/feedback/js/feedback.js (right):

https://codereview.chromium.org/2728773003/diff/40001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:170:
$('user-email-drop-down').options[
On 2017/03/02 19:23:33, xiyuan wrote:
> feedbackInfo.email = $('user-email-drop-down').value;

Oh, I didn't know we could do that!

Done.

https://codereview.chromium.org/2728773003/diff/40001/chrome/browser/resource...
chrome/browser/resources/feedback/js/feedback.js:355: optionElement,
$('user-email-drop-down').firstChild);
On 2017/03/02 19:23:33, xiyuan wrote:
> oops, did not realized that $('user-email-drop-down') is used twice. In this
> case, we should use a var like you had before. :p
> 
> Or use $('anonymous-user-option').

Using the $('anonymous-user-option') instead.

[commit-bot: I haz the power, 2017-03-02 19:56:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xiyuan, 2017-03-02 20:19:38]

lgtm

Cool.

[afakhry, 2017-03-02 20:54:44]

The CQ bit was unchecked by afakhry@chromium.org

[afakhry, 2017-03-02 20:54:49]

The CQ bit was checked by afakhry@chromium.org

[commit-bot: I haz the power, 2017-03-02 20:55:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 22:06:50]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1488488089052030,
"parent_rev": "54d3ba6d6186cd35ef2b124d8c65eab7b2d7cb5e", "commit_rev":
"6795dfa80c19f7e44dc58c37f594b403dab9326e"}

[commit-bot: I haz the power, 2017-03-02 22:07:37]

Description was changed from

==========
Make email field in Feedback app uneditable

Change the text field to a drop down with an option to let users report
anonymously.
Also make the minimize and close buttons recognizable by ChromeVox

SCREEN_SHOT=https://screenshot.googleplex.com/5nmVg5dyUdG.png
BUG=679958
TEST=browser_tests --gtest_filter=FeedbackTest.AnonymousUser
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Make email field in Feedback app uneditable

Change the text field to a drop down with an option to let users report
anonymously.
Also make the minimize and close buttons recognizable by ChromeVox

SCREEN_SHOT=https://screenshot.googleplex.com/5nmVg5dyUdG.png
BUG=679958
TEST=browser_tests --gtest_filter=FeedbackTest.AnonymousUser
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2728773003
Cr-Commit-Position: refs/heads/master@{#454396}
Committed:
https://chromium.googlesource.com/chromium/src/+/6795dfa80c19f7e44dc58c37f594...
==========

[commit-bot: I haz the power, 2017-03-02 22:07:38]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/6795dfa80c19f7e44dc58c37f594...