[Files.app] Not to capture mouse events when the suggest app dialog is visible.

[Files.app] Not to capture mouse events when the suggest app dialog is visible.

r220909 has fixed the share dialog. This patch fixes not oly the share dialog, but also the suggest app dialog and default task pick dialog.

And this patch also introduces FileManagerDialogBase, which manages the dialog in file manager not to have the multiple dialogs visible simultaneously.

BUG=279214
TEST=manually tested
R=hirono@chromium.org

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

[yoshiki, 2013-09-03 08:09:16]

@hirono, PTAL. Thanks.

[hirono, 2013-09-03 08:23:07]

https://codereview.chromium.org/23483029/diff/1/chrome/browser/resources/file...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://codereview.chromium.org/23483029/diff/1/chrome/browser/resources/file...
chrome/browser/resources/file_manager/js/file_manager.js:2360:
this.enableDragOnTitleArea_(false);
Can we make sure that the both dialogs are not opened at the same time?

[yoshiki, 2013-09-03 09:21:32]

On 2013/09/03 08:23:07, hirono wrote:
>
https://codereview.chromium.org/23483029/diff/1/chrome/browser/resources/file...
> File chrome/browser/resources/file_manager/js/file_manager.js (right):
> 
>
https://codereview.chromium.org/23483029/diff/1/chrome/browser/resources/file...
> chrome/browser/resources/file_manager/js/file_manager.js:2360:
> this.enableDragOnTitleArea_(false);
> Can we make sure that the both dialogs are not opened at the same time?

Yes, only one dialog should be visible.

[yoshiki, 2013-09-06 06:25:55]

@hirono, PTAL. I introduced FileManagerDialogBase not to show  multiple dialogs
at the same time.

[hirono, 2013-09-06 07:09:03]

https://codereview.chromium.org/23483029/diff/12001/chrome/browser/resources/...
File chrome/browser/resources/file_manager/js/ui/file_manager_dialog_base.js
(right):

https://codereview.chromium.org/23483029/diff/12001/chrome/browser/resources/...
chrome/browser/resources/file_manager/js/ui/file_manager_dialog_base.js:73:
FileManagerDialogBase.fileManager_.enableDragOnTitleArea_(false);
FileManagerDialogBase looks like one of the UI component.
I expect that the class manages only the dialog and does not touch the outer
environment of the class.
I think this class should not have the responsibility of enableDragOnTitleArea
flag management .
WDYT?

[yoshiki, 2013-09-06 07:48:02]

https://codereview.chromium.org/23483029/diff/12001/chrome/browser/resources/...
File chrome/browser/resources/file_manager/js/ui/file_manager_dialog_base.js
(right):

https://codereview.chromium.org/23483029/diff/12001/chrome/browser/resources/...
chrome/browser/resources/file_manager/js/ui/file_manager_dialog_base.js:73:
FileManagerDialogBase.fileManager_.enableDragOnTitleArea_(false);
On 2013/09/06 07:09:03, hirono wrote:
> FileManagerDialogBase looks like one of the UI component.
> I expect that the class manages only the dialog and does not touch the outer
> environment of the class.
> I think this class should not have the responsibility of enableDragOnTitleArea
> flag management .
> WDYT?

I don't think so. FileManagerDialogBase is the best person to have the
responsibility of enable/disable dragging, because all the dialog uses this
method to show itself. Adding a pair of enableDragOnTitleArea_(false) and
enableDragOnTitleArea_(true) to each caller of show() is mess and may cause a
bug if we forget to write one of the pair.

But I think the name of "enableDragOnTitleArea" was not proper here and I
changed it.

[hirono, 2013-09-06 08:30:13]

lgtm! Thanks

https://chromiumcodereview.appspot.com/23483029/diff/28001/chrome/browser/res...
File chrome/browser/resources/file_manager/js/ui/file_manager_dialog_base.js
(right):

https://chromiumcodereview.appspot.com/23483029/diff/28001/chrome/browser/res...
chrome/browser/resources/file_manager/js/ui/file_manager_dialog_base.js:23: *
The FileManager object.
nit: Could you please add the description about what the variable is used for
and it is OK stay it null in the unit that is created someday.

[yoshiki, 2013-09-09 01:50:41]

Thanks!

https://chromiumcodereview.appspot.com/23483029/diff/28001/chrome/browser/res...
File chrome/browser/resources/file_manager/js/ui/file_manager_dialog_base.js
(right):

https://chromiumcodereview.appspot.com/23483029/diff/28001/chrome/browser/res...
chrome/browser/resources/file_manager/js/ui/file_manager_dialog_base.js:23: *
The FileManager object.
On 2013/09/06 08:30:14, hirono wrote:
> nit: Could you please add the description about what the variable is used for
> and it is OK stay it null in the unit that is created someday.

Done.

[commit-bot: I haz the power, 2013-09-09 01:50:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/23483029/30001

[commit-bot: I haz the power, 2013-09-09 02:10:04]

Retried try job too often on linux_chromeos for step(s) app_list_unittests,
ash_unittests, aura_unittests, base_unittests, browser_tests,
cacheinvalidation_unittests, check_deps, chromeos_unittests,
components_unittests, content_browsertests, content_unittests, crypto_unittests,
dbus_unittests, device_unittests, gpu_unittests, interactive_ui_tests,
ipc_tests, jingle_unittests, media_unittests, net_unittests, ppapi_unittests,
printing_unittests, sandbox_linux_unittests, sql_unittests, sync_unit_tests,
unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-09-09 04:20:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/23483029/57001

[commit-bot: I haz the power, 2013-09-09 04:39:30]

Retried try job too often on linux_chromeos_clang for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-09-09 05:39:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/23483029/79001

[commit-bot: I haz the power, 2013-09-09 14:48:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/23483029/95001

[commit-bot: I haz the power, 2013-09-09 14:48:23]

Failed to apply patch for chrome/browser/resources/file_manager/main.html:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/resources/file_manager/main.html
  Hunk #3 FAILED at 114.
  1 out of 3 hunks FAILED -- saving rejects to file
chrome/browser/resources/file_manager/main.html.rej

Patch:       chrome/browser/resources/file_manager/main.html
Index: chrome/browser/resources/file_manager/main.html
diff --git a/chrome/browser/resources/file_manager/main.html
b/chrome/browser/resources/file_manager/main.html
index
1874116fe1b789b102114a4189fa113e4e802d37..2831ee267c05e4b16ff3c586f8ee382064827696
100644
--- a/chrome/browser/resources/file_manager/main.html
+++ b/chrome/browser/resources/file_manager/main.html
@@ -81,6 +81,7 @@
 
       <script src="js/combobutton.js"></script>
       <script src="js/commandbutton.js"></script>
+      <script src="js/ui/file_manager_dialog_base.js"></script>
 
       <script src="js/app_installer.js"></script>
       <script src="js/async_util.js"></script>
@@ -98,7 +99,6 @@
       <script src="js/error_dialog.js"></script>
       <script src="js/file_operation_manager_wrapper.js"></script>
       <script src="js/file_grid.js"></script>
-      <script src="js/file_manager.js"></script>
       <script src="js/file_manager_pyauto.js"></script>
       <script src="js/file_selection.js"></script>
       <script src="js/file_table.js"></script>
@@ -114,6 +114,7 @@
       <script src="js/suggest_apps_dialog.js"></script>
       <script src="js/text_measure.js"></script>
       <script src="js/tree.css.js"></script>
+      <script src="js/ui/file_manager.js"></script>
       <script src="js/url_constants.js"></script>
       <script src="js/volume_manager.js"></script>
       <script src="js/media/media_util.js"></script>

[yoshiki, 2013-09-10 01:49:38]

Committed patchset #5 manually as r222174 (presubmit successful).