New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 789226 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Files app: Unable to touch and open files after opening first file using double tap

Project Member Reported by sdantul...@chromium.org, Nov 28 2017

Issue description

Google Chrome	64.0.3274.0 (Official Build) dev (64-bit)
Revision	0
Platform	10150.0.0 (Official Build) dev-channel eve

What steps will reproduce the problem?
1. Save some image files in Downloads folder
2. Double tap to open an image file. File opens in Gallery app
3. Switch to Files app and touch to open any other file

What is the expected result?
Image should open in Gallery app.

What happens instead?
Nothing happens. Flicker of 'OPEN' button is seen in the menu bar
 
Summary: Files app: Unable to double tap to open and view files (using touch) (was: Files app: Unable to double tap to open and view files)
Description: Show this description
Summary: Files app: Unable to touch and open files after opening first file using double tap (was: Files app: Unable to double tap to open and view files (using touch))
Cc: kathrelk...@chromium.org
 Issue 789229  has been merged into this issue.
Following M62 - Touch functionality has been updated such that files should open on a single press rather than a double-tap.

I am unable to reproduce this on an M64 Dev channel Samus. Could you please double check?
weifangsun@ Confirmed that single press works perfectly fine.

Issue is observed when I accidentally double tap on a file to open it and then use single press to open subsequent files. Tested on Samus M64 dev channel.
Owner: yamaguchi@chromium.org
Status: Assigned (was: Untriaged)
Ahh, I see. Thanks for clarifying!

yamaguchi@ - Looks like a weird race condition/other action triggered from the unexpected double tap that's causing this behavior. Could you please take a look?


Status: Started (was: Assigned)
Investigating now. This looks related to the apps rather than tap handling.

I could also reproduce by clicking "OPEN" button with mouse many times very quickly.
Once this bug triggers, the Gallery app doesn't pop up anymore regardless of if it's by tap, double click, context menu, or the menu bar button. It can be restored after while, or signing out once. Other apps (e.g. Google Keep) can still open the file.

The same issue also happens with the audio player app (e.g. .mp3 file).
Cc: yoshiki@chromium.org
The background page of the Gallery app crashes at launch when this is happening.
https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/background/js/app_window_wrapper.js?q=SingletonAppWindowWrapper&sq=package:chromium&l=299
> Uncaught TypeError: this.window_.contentWindow.reload is not a function.

https://chromium-review.googlesource.com/c/chromium/src/+/800910 is a quick and tentative fix. It could drop reloading of the reused window in some limited edge cases.

The reload() method is not ready until loaded (onload).
https://developer.chrome.com/apps/app_window#method-create
AppWindowWrapper currently uses a single queue for async tasks, and currently the .launch() doesn't block until the created window finishes loading. Though I think we shouldn't make the single queue wait window loaded for performance reason. An ideal solution would be to extend the task management logic so that it could parallelize such different dependencies.
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd2ed94fb601a586e28c17298c32c5e6c89b3e50

commit dd2ed94fb601a586e28c17298c32c5e6c89b3e50
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Wed Dec 06 04:16:15 2017

Avoid runtime error when window.reload is not available.

appWindow object doesn't have reload() method ready until the window is
loaded. This is a quick fix to avoid crashing the background script by
user operation.

Test: manually verified as noted in the bug
Bug:  789226 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ie2266792d8ca62bd0bae74ff2824b72221acbdb7
Reviewed-on: https://chromium-review.googlesource.com/800910
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522000}
[modify] https://crrev.com/dd2ed94fb601a586e28c17298c32c5e6c89b3e50/ui/file_manager/file_manager/background/js/app_window_wrapper.js

Labels: Merge-Request-64
Labels: -Merge-Request-64 Merge-Approved-64
Approving merge to M64 Chrome OS.
Project Member

Comment 16 by bugdroid1@chromium.org, Dec 11 2017

Labels: -merge-approved-64 merge-merged-3282
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/242e50f588e6a59bcdd198a70cabb8a2acbebdfc

commit 242e50f588e6a59bcdd198a70cabb8a2acbebdfc
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Mon Dec 11 02:39:12 2017

Avoid runtime error when window.reload is not available.

appWindow object doesn't have reload() method ready until the window is
loaded. This is a quick fix to avoid crashing the background script by
user operation.

Test: manually verified as noted in the bug
Bug:  789226 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ie2266792d8ca62bd0bae74ff2824b72221acbdb7
Reviewed-on: https://chromium-review.googlesource.com/800910
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#522000}(cherry picked from commit dd2ed94fb601a586e28c17298c32c5e6c89b3e50)
Reviewed-on: https://chromium-review.googlesource.com/813334
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#121}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/242e50f588e6a59bcdd198a70cabb8a2acbebdfc/ui/file_manager/file_manager/background/js/app_window_wrapper.js

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
10176.30.0, 64.0.3282.57

Sign in to add a comment