New issue
Advanced search Search tips

Issue 918808 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: File name and Save button fields are seen disabled in Save page overlay for first time

Project Member Reported by rkalavakuntla@chromium.org, Jan 3

Issue description

Chrome Version:73.0.3654.0/11515.0.0 dev channel Daisy,Kip,Reks
OS: chrome OS

What steps will reproduce the problem?
(1)Sign into user >> launch chrome,save any page and observe

Actual: File name and Save button fields are seen disabled in Save page overlay
Expected:File name and Save button fields should be seen enabled when a page is saved.

Note: Issue is seen only for first time, works fine when we save second time

This is a Regression issue as same works fine in 72.0.3626.30/11316.35.0 dev

Attached the screencast for reference..
 
Actual.mp4
8.9 MB View Download
Cc: slangley@chromium.org lucmult@chromium.org
Labels: CrOSFilesCategory-UI
Can be repro'd at at time by opening the task manager and killing the Files app background page. The following exception can be seen in the foreground page for the open dialog:

[unhandled-error]: TypeError: Cannot read property 'filesystem' of undefined
Error: original stack
    at console.error.args [as error] (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:68:19)
    at window.addEventListener (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js:51:11)
console.error.args @ main_scripts.js:71
chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js:5484 Uncaught (in promise) TypeError: Cannot read property 'filesystem' of undefined
    at VolumeManagerImpl.getVolumeInfo (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js:5484)
    at VolumeManagerImpl.getLocationInfo (chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/background/js/background_common_scripts.js:5510)
    at FilteredVolumeManager.getLocationInfo (main_scripts.js:53131)
    at HTMLDivElement.EntryListItem.sortEntries (main_scripts.js:43593)
    at reader.readEntries (main_scripts.js:43627)
    at CombinedReaders.readEntries (main_scripts.js:13502)
    at readEntry (main_scripts.js:43625)
    at HTMLDivElement.EntryListItem.updateSubDirectories (main_scripts.js:43640)
    at HTMLUnknownElement.DirectoryTree.updateSubElementsFromList (main_scripts.js:44650)
    at HTMLUnknownElement.DirectoryTree.redraw (main_scripts.js:44841)
8main_scripts.js:10500 Uncaught TypeError: command.matchesEvent is not a function
    at CommandManager.handleKeyDown_ (main_scripts.js:10500)
Cc: -lucmult@chromium.org
Owner: lucmult@chromium.org
Status: Assigned (was: Untriaged)
Disabling MyFiles fixes this issue.
Cc: lucmult@chromium.org
Owner: sa...@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 9

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

commit 63546ce7e83ce49fa4cf52cd8f5591421b4ace89
Author: Sam McNally <sammc@chromium.org>
Date: Wed Jan 09 06:06:04 2019

Eagerly resolve display roots - synchronously for non-Drive volumes.

When launching a select file dialog without the Files app already
running, the VolumeEntry instances are initialized concurrently in the
background page with the foreground dialog. If file type filters are
requested, they trigger a redraw before the root entry for the
My files VolumeEntry has resolved.

If My files contains no other volumes (Play files or Linux files) and
its root entry isn't ready, reading its contents returns no files. The
code responsible for sorting those entries assumes there should always
be at least one entry (since Downloads should always exist), causing an
uncaught exception, leaving the UI in a broken state.

Fix this by eagerly resolving the display entry in VolumeInfoImpl and,
where possible (non-Drive volumes), populate the VolumeInfoImpl display
root field synchronously.

Bug:  918808 
Change-Id: I1be0b5574a923e0302185b92efdf19c8f9e73be5
Reviewed-on: https://chromium-review.googlesource.com/c/1400421
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621059}
[modify] https://crrev.com/63546ce7e83ce49fa4cf52cd8f5591421b4ace89/ui/file_manager/file_manager/background/js/volume_info_impl.js
[modify] https://crrev.com/63546ce7e83ce49fa4cf52cd8f5591421b4ace89/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.js

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 9

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

commit 4bea48cf31dab6ff32d680c064e5ef8364aa4c30
Author: Sam McNally <sammc@chromium.org>
Date: Wed Jan 09 07:30:39 2019

Run SelectFileDialogExtensionBrowserTest tests with a file type filter.

Supplying a file type filter causes an extra redraw early on,
potentially triggering different functionality to without. Also, ensure
the background page has shut down before opening a dialog.

Bug:  918808 
Change-Id: I78e2fba8225b5c33d958386fc811c08b50b45512
Reviewed-on: https://chromium-review.googlesource.com/c/1401828
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621067}
[modify] https://crrev.com/4bea48cf31dab6ff32d680c064e5ef8364aa4c30/chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 10

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

commit 5a4a120922072e3e91cde73bad8c6d17565583c5
Author: Sam McNally <sammc@chromium.org>
Date: Thu Jan 10 07:01:18 2019

Deflake files app tests covering launching with volumes unmounted.

Starting with volumes mounted and then unmounting them sometimes causes
an uncaught exception while resolving the My Drive root. Avoid this by
adding an option for files app tests to start without any volumes
mounted and update the tests to add volumes as needed.

Bug:  918808 
Change-Id: I3f6fc59d097750d7b3d10c9ed227ff3a71ec2f17
Reviewed-on: https://chromium-review.googlesource.com/c/1404541
Commit-Queue: Sam McNally <sammc@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621490}
[modify] https://crrev.com/5a4a120922072e3e91cde73bad8c6d17565583c5/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/5a4a120922072e3e91cde73bad8c6d17565583c5/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/5a4a120922072e3e91cde73bad8c6d17565583c5/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h
[modify] https://crrev.com/5a4a120922072e3e91cde73bad8c6d17565583c5/ui/file_manager/integration_tests/file_manager/file_display.js

Status: Fixed (was: Assigned)

Sign in to add a comment