Bookmark context menu "Add folder..." allows to create folder with empty name(empty string).

Bookmark context menu "Add folder..." allows to create folder with empty name(empty string).

If user enters empty string in textField of New folder dialog window, just default the name
to "New folder" instead of creating folder with empty name.

BUG=398852
R=asvitkine@chromium.org

TEST=
1) Right click on bookmark bar, and select "Add folder..." from context menu.
2) In the textField, do not enter any name (or delete existing text).
3) Click on save button and observe.
4) Newly created folder name must default to "New folder" instead of empty name(empty string).

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

[siba.samal, 2014-08-14 08:04:09]

This CL is related to BUG #398852. It does fix similar issue but not the actual
issue.

PTAL

[Alexei Svitkine (slow), 2014-08-14 19:27:12]

Please add a TEST= line to the CL description that can be used by QA to verify
the fix. Thanks!

Other than above and below comments, LGTM. Thanks!

https://codereview.chromium.org/476643002/diff/1/chrome/browser/ui/cocoa/book...
File chrome/browser/ui/cocoa/bookmarks/bookmark_name_folder_controller.mm
(right):

https://codereview.chromium.org/476643002/diff/1/chrome/browser/ui/cocoa/book...
chrome/browser/ui/cocoa/bookmarks/bookmark_name_folder_controller.mm:100: if
(![name length])
Nit: == 0 instead

[siba.samal, 2014-08-16 05:06:05]

The CQ bit was checked by siba.samal@samsung.com

[commit-bot: I haz the power, 2014-08-16 05:07:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siba.samal@samsung.com/476643002/20001

[siba.samal, 2014-08-16 05:30:47]

https://codereview.chromium.org/476643002/diff/1/chrome/browser/ui/cocoa/book...
File chrome/browser/ui/cocoa/bookmarks/bookmark_name_folder_controller.mm
(right):

https://codereview.chromium.org/476643002/diff/1/chrome/browser/ui/cocoa/book...
chrome/browser/ui/cocoa/bookmarks/bookmark_name_folder_controller.mm:100: if
(![name length])
On 2014/08/14 19:27:11, Alexei Svitkine wrote:
> Nit: == 0 instead

Done.

[commit-bot: I haz the power, 2014-08-16 09:37:18]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-16 09:40:20]

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

[commit-bot: I haz the power, 2014-08-16 09:40:21]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[siba.samal, 2014-08-16 09:42:28]

The CQ bit was checked by siba.samal@samsung.com

[commit-bot: I haz the power, 2014-08-16 09:43:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siba.samal@samsung.com/476643002/20001

[commit-bot: I haz the power, 2014-08-16 10:20:52]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-16 10:23:28]

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

[commit-bot: I haz the power, 2014-08-16 10:23:29]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[siba.samal, 2014-08-16 10:30:44]

The CQ bit was checked by siba.samal@samsung.com

[commit-bot: I haz the power, 2014-08-16 10:31:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siba.samal@samsung.com/476643002/20001

[commit-bot: I haz the power, 2014-08-16 10:34:08]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-16 10:37:51]

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

[commit-bot: I haz the power, 2014-08-16 10:37:55]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[siba.samal, 2014-08-16 16:58:19]

The CQ bit was checked by siba.samal@samsung.com

[commit-bot: I haz the power, 2014-08-16 16:58:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siba.samal@samsung.com/476643002/20001

[commit-bot: I haz the power, 2014-08-16 17:01:17]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-16 17:04:15]

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

[commit-bot: I haz the power, 2014-08-16 17:04:16]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[siba.samal, 2014-08-16 17:11:06]

The CQ bit was unchecked by siba.samal@samsung.com

[siba.samal, 2014-08-16 17:11:06]

The CQ bit was checked by siba.samal@samsung.com

[commit-bot: I haz the power, 2014-08-16 17:11:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siba.samal@samsung.com/476643002/20001

[commit-bot: I haz the power, 2014-08-16 17:14:08]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-16 17:16:59]

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

[commit-bot: I haz the power, 2014-08-16 17:17:00]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[siba.samal, 2014-08-16 17:45:46]

The CQ bit was checked by siba.samal@samsung.com

[commit-bot: I haz the power, 2014-08-16 17:46:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siba.samal@samsung.com/476643002/20001

[commit-bot: I haz the power, 2014-08-16 17:49:24]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-16 17:51:45]

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

[commit-bot: I haz the power, 2014-08-16 17:51:46]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[tfarina, 2014-08-16 21:23:36]

Looks like you have to sign the CLA and add your name to AUTHORS file:

siba.samal@samsung.com is not in AUTHORS file. If you are a new contributor,
please visit
http://www.chromium.org/developers/contributing-code and read the "Legal"
section
If you are a chromite, verify the contributor signed the CLA.

[siba.samal, 2014-08-17 01:36:12]

On 2014/08/16 21:23:36, tfarina wrote:
> Looks like you have to sign the CLA and add your name to AUTHORS file:
> 
> mailto:siba.samal@samsung.com is not in AUTHORS file. If you are a new
contributor,
> please visit
> http://www.chromium.org/developers/contributing-code and read the "Legal"
> section
> If you are a chromite, verify the contributor signed the CLA.

I have signed CLA but not added in AUTHORS file.
Submitted patch for the same, waiting for approval.
https://codereview.chromium.org/484433002/

[siba.samal, 2014-08-18 09:20:06]

The CQ bit was checked by siba.samal@samsung.com

[commit-bot: I haz the power, 2014-08-18 09:20:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siba.samal@samsung.com/476643002/40001

[commit-bot: I haz the power, 2014-08-18 11:08:03]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-18 11:10:42]

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

[commit-bot: I haz the power, 2014-08-18 11:10:43]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[siba.samal, 2014-08-18 11:15:50]

The CQ bit was checked by siba.samal@samsung.com

[commit-bot: I haz the power, 2014-08-18 11:16:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siba.samal@samsung.com/476643002/60001

[commit-bot: I haz the power, 2014-08-18 12:04:48]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)

[siba.samal, 2014-08-18 12:42:29]

The CQ bit was unchecked by siba.samal@samsung.com

[siba.samal, 2014-08-18 12:43:56]

The CQ bit was checked by siba.samal@samsung.com

[commit-bot: I haz the power, 2014-08-18 12:44:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siba.samal@samsung.com/476643002/80001

[commit-bot: I haz the power, 2014-08-18 14:30:58]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-18 15:43:25]

Committed patchset #5 (80001) as 290262