Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2522403002:
Show the leaf folder name as the window name. (Closed)
|
Created:
4 years ago by yamaguchi Modified:
4 years ago Reviewers:
fukino CC:
chromium-reviews, oka+watch_chromium.org, rginda+watch_chromium.org, mtomasz+watch_chromium.org, yamaguchi+watch_chromium.org, fukino+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionShow the leaf folder name as the window name.
BUG=650490
TEST=manual test as noted in the bug
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/894d7fec2a4b445294e2a4d132a0ae4d9bf6f494
Cr-Commit-Position: refs/heads/master@{#434613}
Patch Set 1 #Patch Set 2 : Resolve JS compilation errors. #Patch Set 3 : Add comment. #
Total comments: 18
Patch Set 4 : Retrieve VolumeInfo in the helper function. #
Total comments: 2
Patch Set 5 : Clarify special path for root folders. #
Total comments: 11
Patch Set 6 : Use getRootTypeLabel for root entries. #Patch Set 7 : Simplify the logic. #Patch Set 8 : Handle error where locationInfo is not found. #Patch Set 9 : Insert space between an error message and value. #
Total comments: 4
Patch Set 10 : Style fix. #Messages
Total messages: 53 (38 generated)
Description was changed from ========== Show the leaf folder name as the window name. BUG=650490 ========== to ========== Show the leaf folder name as the window name. BUG=650490 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) closure_compilation on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Show the leaf folder name as the window name. BUG=650490 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Show the leaf folder name as the window name. BUG=650490 TEST=manual test as noted in the bug CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
yamaguchi@chromium.org changed reviewers: + fukino@chromium.org
ptal
https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... File ui/file_manager/file_manager/common/js/util.js (right): https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/common/js/util.js:924: * @param {string} rootType Root type in Location info. @param {VolumeManagerCommon.RootType} https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:399: newVolumeInfo) { We need a function 'DirectoryEntry -> DocumentTitle', so maybe it's simpler to drop newVolumeInfo from this function's parameter and call this.volumeManager_.getVolumeInfo(dirEntry) in this function. I'm a bit confused about the difference between newVolumeInfo and a volumeInfo which is retrieved from dirEntry. https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:400: if (dirEntry.rootType) needs '{' for multi-line if statements. https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:400: if (dirEntry.rootType) Use util.isFakeEntry(). https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:402: return util.getRootTypeLabelForRootType(dirEntry.rootType) || I think we don't need util.getRootTypeLabel(volumeManager.getLocationInfo(entry)) should work. (without adding util.getRootTypeLabelForRootType()). https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:407: var leafName; Can't you use Entry.name? https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:448: /** @type {DirectoryEntry} */ (event.newDirEntry), newVolumeInfo); If event.newDirEntry is not guaranteed to be non-null, you should check "if (event.newDirEntry)..." or getWindowName_ should handle null.
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:399: newVolumeInfo) { On 2016/11/25 08:02:50, fukino wrote: > We need a function 'DirectoryEntry -> DocumentTitle', so maybe it's simpler to > drop newVolumeInfo from this function's parameter and call > this.volumeManager_.getVolumeInfo(dirEntry) in this function. > I'm a bit confused about the difference between newVolumeInfo and a volumeInfo > which is retrieved from dirEntry. Done. https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:400: if (dirEntry.rootType) On 2016/11/25 08:02:50, fukino wrote: > Use util.isFakeEntry(). Done. https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:402: return util.getRootTypeLabelForRootType(dirEntry.rootType) || On 2016/11/25 08:02:50, fukino wrote: > I think we don't need > util.getRootTypeLabel(volumeManager.getLocationInfo(entry)) should work. > (without adding util.getRootTypeLabelForRootType()). Done. https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:407: var leafName; On 2016/11/25 08:02:50, fukino wrote: > Can't you use Entry.name? Done. However, it seems the field is also set to "root" for the / folder of Google Drive. https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:448: /** @type {DirectoryEntry} */ (event.newDirEntry), newVolumeInfo); On 2016/11/25 08:02:50, fukino wrote: > If event.newDirEntry is not guaranteed to be non-null, you should check "if > (event.newDirEntry)..." or getWindowName_ should handle null. Done. If event.newDirEntry is invalid, it should not update the UI.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:407: var leafName; On 2016/11/25 09:11:58, yamaguchi wrote: > On 2016/11/25 08:02:50, fukino wrote: > > Can't you use Entry.name? > > Done. > However, it seems the field is also set to "root" for the / folder of Google > Drive. What label do you show for the root directory of Google drive? I suppose it is "My Drive". If so, let's handle the case root directory as a special case and avoid splitting the path to make the intention clear. How about something like: locationInfo.isRootEntry ? util.getRootLabel(locationInfo) : entry.name https://codereview.chromium.org/2522403002/diff/10002/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/10002/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:447: if (this.dialogType_ === DialogType.FULL_PAGE && newVolumeInfo) { && newVolumeInfo looks unnecessary.
https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:407: var leafName; On 2016/11/25 10:30:22, fukino wrote: > On 2016/11/25 09:11:58, yamaguchi wrote: > > On 2016/11/25 08:02:50, fukino wrote: > > > Can't you use Entry.name? > > > > Done. > > However, it seems the field is also set to "root" for the / folder of Google > > Drive. > > What label do you show for the root directory of Google drive? > I suppose it is "My Drive". > > If so, let's handle the case root directory as a special case and avoid > splitting the path to make the intention clear. > > How about something like: > locationInfo.isRootEntry ? util.getRootLabel(locationInfo) : entry.name And I think the expression above covers the fake entry cases. (So line 400-403 can be removed) If locationInfo.isRoot is not true for fake entries, please check locationInfo.isSpecialSearchRoot.
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:407: var leafName; On 2016/11/25 10:30:22, fukino wrote: > On 2016/11/25 09:11:58, yamaguchi wrote: > > On 2016/11/25 08:02:50, fukino wrote: > > > Can't you use Entry.name? > > > > Done. > > However, it seems the field is also set to "root" for the / folder of Google > > Drive. > > What label do you show for the root directory of Google drive? > I suppose it is "My Drive". > > If so, let's handle the case root directory as a special case and avoid > splitting the path to make the intention clear. > > How about something like: > locationInfo.isRootEntry ? util.getRootLabel(locationInfo) : entry.name Done. It was "Google Drive" in the older revision, but now fixed it to be "My Drive". https://codereview.chromium.org/2522403002/diff/10002/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/10002/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:447: if (this.dialogType_ === DialogType.FULL_PAGE && newVolumeInfo) { On 2016/11/25 10:30:22, fukino wrote: > && newVolumeInfo looks unnecessary. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:407: var leafName; On 2016/11/25 10:44:39, fukino wrote: > On 2016/11/25 10:30:22, fukino wrote: > > On 2016/11/25 09:11:58, yamaguchi wrote: > > > On 2016/11/25 08:02:50, fukino wrote: > > > > Can't you use Entry.name? > > > > > > Done. > > > However, it seems the field is also set to "root" for the / folder of Google > > > Drive. > > > > What label do you show for the root directory of Google drive? > > I suppose it is "My Drive". > > > > If so, let's handle the case root directory as a special case and avoid > > splitting the path to make the intention clear. > > > > How about something like: > > locationInfo.isRootEntry ? util.getRootLabel(locationInfo) : entry.name > > And I think the expression above covers the fake entry cases. (So line 400-403 > can be removed) > > If locationInfo.isRoot is not true for fake entries, please check > locationInfo.isSpecialSearchRoot. How about this comment? https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:393: * @return {?string} window name. null if the location info for |dirEntry| is Is there a path which returns null? https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:399: if (util.isFakeEntry(dirEntry)) { if (util.isFakeEntry(dirEntry)) return util.getRootTypeLabel(locationInfo); should work. https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:405: ? util.getEntryLabel(locationInfo, For root entries, you can use util.getRootTypeLabel(). https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:441: if (newWindowName) If getWindowName_ doesn't return null, please remove this if statement.
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:393: * @return {?string} window name. null if the location info for |dirEntry| is On 2016/11/28 00:46:25, fukino wrote: > Is there a path which returns null? Done. getEntryLabel had returned ?string, but now we use getRootTypeLabel instead. https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:399: if (util.isFakeEntry(dirEntry)) { On 2016/11/28 00:46:25, fukino wrote: > if (util.isFakeEntry(dirEntry)) > return util.getRootTypeLabel(locationInfo); > > should work. https://cs.chromium.org/chromium/src/ui/file_manager/externs/volume_manager.j... locationInfo from getLocationInfo can be null. https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:405: ? util.getEntryLabel(locationInfo, On 2016/11/28 00:46:25, fukino wrote: > For root entries, you can use util.getRootTypeLabel(). Done.
https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:407: var leafName; On 2016/11/28 00:46:25, fukino wrote: > On 2016/11/25 10:44:39, fukino wrote: > > On 2016/11/25 10:30:22, fukino wrote: > > > On 2016/11/25 09:11:58, yamaguchi wrote: > > > > On 2016/11/25 08:02:50, fukino wrote: > > > > > Can't you use Entry.name? > > > > > > > > Done. > > > > However, it seems the field is also set to "root" for the / folder of > Google > > > > Drive. > > > > > > What label do you show for the root directory of Google drive? > > > I suppose it is "My Drive". > > > > > > If so, let's handle the case root directory as a special case and avoid > > > splitting the path to make the intention clear. > > > > > > How about something like: > > > locationInfo.isRootEntry ? util.getRootLabel(locationInfo) : entry.name > > > > And I think the expression above covers the fake entry cases. (So line 400-403 > > can be removed) > > > > If locationInfo.isRoot is not true for fake entries, please check > > locationInfo.isSpecialSearchRoot. > > How about this comment? Ping. https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:399: if (util.isFakeEntry(dirEntry)) { On 2016/11/28 02:51:13, yamaguchi wrote: > On 2016/11/28 00:46:25, fukino wrote: > > if (util.isFakeEntry(dirEntry)) > > return util.getRootTypeLabel(locationInfo); > > > > should work. > > https://cs.chromium.org/chromium/src/ui/file_manager/externs/volume_manager.j... > locationInfo from getLocationInfo can be null. The return value of getVolumeInfo() can be null too, so newVolumeInfo.label does not handle the error case. How about doing like this in the beginning of this function if (!locationInfo) { console.error({some error message}); return ''; } https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:441: if (newWindowName) On 2016/11/28 00:46:25, fukino wrote: > If getWindowName_ doesn't return null, please remove this if statement. Ping.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/40001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:407: var leafName; On 2016/11/28 03:12:23, fukino wrote: > On 2016/11/28 00:46:25, fukino wrote: > > On 2016/11/25 10:44:39, fukino wrote: > > > On 2016/11/25 10:30:22, fukino wrote: > > > > On 2016/11/25 09:11:58, yamaguchi wrote: > > > > > On 2016/11/25 08:02:50, fukino wrote: > > > > > > Can't you use Entry.name? > > > > > > > > > > Done. > > > > > However, it seems the field is also set to "root" for the / folder of > > Google > > > > > Drive. > > > > > > > > What label do you show for the root directory of Google drive? > > > > I suppose it is "My Drive". > > > > > > > > If so, let's handle the case root directory as a special case and avoid > > > > splitting the path to make the intention clear. > > > > > > > > How about something like: > > > > locationInfo.isRootEntry ? util.getRootLabel(locationInfo) : entry.name > > > > > > And I think the expression above covers the fake entry cases. (So line > 400-403 > > > can be removed) > > > > > > If locationInfo.isRoot is not true for fake entries, please check > > > locationInfo.isSpecialSearchRoot. > > > > How about this comment? > > Ping. Done. https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:399: if (util.isFakeEntry(dirEntry)) { On 2016/11/28 03:12:23, fukino wrote: > On 2016/11/28 02:51:13, yamaguchi wrote: > > On 2016/11/28 00:46:25, fukino wrote: > > > if (util.isFakeEntry(dirEntry)) > > > return util.getRootTypeLabel(locationInfo); > > > > > > should work. > > > > > https://cs.chromium.org/chromium/src/ui/file_manager/externs/volume_manager.j... > > locationInfo from getLocationInfo can be null. > > The return value of getVolumeInfo() can be null too, so newVolumeInfo.label does > not handle the error case. > > How about doing like this in the beginning of this function > > if (!locationInfo) { > console.error({some error message}); > return ''; > } Done. https://codereview.chromium.org/2522403002/diff/70001/ui/file_manager/file_ma... ui/file_manager/file_manager/foreground/js/main_window_component.js:441: if (newWindowName) On 2016/11/28 03:12:23, fukino wrote: > On 2016/11/28 00:46:25, fukino wrote: > > If getWindowName_ doesn't return null, please remove this if statement. > > Ping. Changed to handle the case where locationInfo is not found instead.
lgtm with nits. https://codereview.chromium.org/2522403002/diff/150001/ui/file_manager/file_m... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/150001/ui/file_manager/file_m... ui/file_manager/file_manager/foreground/js/main_window_component.js:422: return; optional nit: I prefer the following form to avoid return statement in the middle. if (locationInfo) { document.title = ... } else { console.error() } https://codereview.chromium.org/2522403002/diff/150001/ui/file_manager/file_m... ui/file_manager/file_manager/foreground/js/main_window_component.js:428: else nit: else clause needs braces since the if statement has braces now. } else { ... }
The CQ bit was checked by yamaguchi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2522403002/diff/150001/ui/file_manager/file_m... File ui/file_manager/file_manager/foreground/js/main_window_component.js (right): https://codereview.chromium.org/2522403002/diff/150001/ui/file_manager/file_m... ui/file_manager/file_manager/foreground/js/main_window_component.js:422: return; On 2016/11/28 05:26:40, fukino wrote: > optional nit: I prefer the following form to avoid return statement in the > middle. > > if (locationInfo) { > document.title = ... > } else { > console.error() > } Done. https://codereview.chromium.org/2522403002/diff/150001/ui/file_manager/file_m... ui/file_manager/file_manager/foreground/js/main_window_component.js:428: else On 2016/11/28 05:26:40, fukino wrote: > nit: else clause needs braces since the if statement has braces now. > > } else { > ... > } Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by yamaguchi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fukino@chromium.org Link to the patchset: https://codereview.chromium.org/2522403002/#ps170001 (title: "Style fix.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 170001, "attempt_start_ts": 1480314778676730,
"parent_rev": "5e2273f9639b1fcb22e1f99b58b434d6521615a7", "commit_rev":
"09f9fbe6abb97b84b130d934deb5798239e47fd0"}
Message was sent while issue was closed.
Description was changed from ========== Show the leaf folder name as the window name. BUG=650490 TEST=manual test as noted in the bug CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Show the leaf folder name as the window name. BUG=650490 TEST=manual test as noted in the bug CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #10 (id:170001)
Message was sent while issue was closed.
Description was changed from ========== Show the leaf folder name as the window name. BUG=650490 TEST=manual test as noted in the bug CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Show the leaf folder name as the window name. BUG=650490 TEST=manual test as noted in the bug CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/894d7fec2a4b445294e2a4d132a0ae4d9bf6f494 Cr-Commit-Position: refs/heads/master@{#434613} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/894d7fec2a4b445294e2a4d132a0ae4d9bf6f494 Cr-Commit-Position: refs/heads/master@{#434613} |
