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

Issue 762699 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 640685



Sign in to add a comment

Can't enter quick view from a folder

Project Member Reported by mcirimele@chromium.org, Sep 6 2017

Issue description

Chrome Version: 63.0.3206.0 9916.0.0 Kevin

What steps will reproduce the problem?
(1) Focus or select a folder
(2) Press spacebar to open quick view

What is the expected result?
Quick view opens and shows information about the folder.

What happens instead?
Nothing. 

As a side note you can actually get to quick view of a folder by starting on a file and then using the arrow keys to get to a folder. It shows the latest modified by date but no size. 
 
Blocking: 640685
Labels: -M-63

Comment 2 by oka@chromium.org, Sep 25 2017

Strange. It works for me on ToT using Linux.
Does it a specific issue to Kevin?

Comment 3 by oka@chromium.org, Sep 25 2017

Cc: oka@chromium.org

Comment 4 by oka@chromium.org, Sep 25 2017

Cc: fukino@chromium.org
Labels: OS-Chrome
Owner: marian...@google.com
Status: Assigned (was: Untriaged)
OK. I confirmed it on ToT.
The problem is that the key down event which triggered quick view opening can also subsequently trigger quick view closing.
We should call event.stopImmediatePropagation() in onKeyDownToOpen_.
https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/foreground/js/quick_view_controller.js?q=quick_view_controller&sq=package:chromium&dr&l=205 

Marianne, could you take this? It's awesome if you can add a test to prevent regression.

Test files are located in:
ui/file_manager/integration_tests/file_manager/quick_view.js .
cf. https://chromium-review.googlesource.com/c/chromium/src/+/567910

Status: Started (was: Assigned)
Started working on it
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 27 2017

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

commit ed36587b63ac296a5a9cd979142574533d8d18c6
Author: mariannet <mariannet@google.com>
Date: Wed Sep 27 04:08:04 2017

Make sure that folder quick view can be opened reliably.

A race condition sometimes closed the quick view the moment it was opened.
This behavior could be witnessed when rendering the quick view was fast, like
it is for opening folders.
Make sure the <space> key down event only gets handled once (for either opening
or closing) instead of twice, thus eliminating the race condition.
Opening quick view for folders didn't work reliably, fix applied.

Bug:  762699 
Test: manually tested
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I899437e312cfa7ad76d45706b670a2db4c83fc48
Reviewed-on: https://chromium-review.googlesource.com/683498
Reviewed-by: Keigo Oka <oka@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Marianne Thieffry <mariannet@google.com>
Cr-Commit-Position: refs/heads/master@{#504577}
[modify] https://crrev.com/ed36587b63ac296a5a9cd979142574533d8d18c6/ui/file_manager/file_manager/foreground/js/quick_view_controller.js

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 3 2017

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

commit 23d7fb4389c98a96be6607b72fd58a92d5d11fd8
Author: mariannet <mariannet@google.com>
Date: Tue Oct 03 07:46:52 2017

Add a test that opens quick view, closes it, and opens it again on a folder.

Originally, this was supposed to be a regression test for  bug 762699 , but this
test can't detect the bug...
Test that first opens quick view, closes it, then opens it again.

Bug:  762699 
Test: this adds a test
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib3d8ac961dc4fd57a1ec28e53378a5ba92b073f8
Reviewed-on: https://chromium-review.googlesource.com/683715
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Keigo Oka <oka@chromium.org>
Commit-Queue: Marianne Thieffry <mariannet@google.com>
Cr-Commit-Position: refs/heads/master@{#505978}
[modify] https://crrev.com/23d7fb4389c98a96be6607b72fd58a92d5d11fd8/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/23d7fb4389c98a96be6607b72fd58a92d5d11fd8/ui/file_manager/integration_tests/file_manager/quick_view.js

Comment 9 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 10 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment