Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(318)

Issue 415403006: [Files App] Only use the first matching file handler from each app. (Closed)

Created:
6 years, 4 months ago by jackhou1
Modified:
6 years, 4 months ago
Reviewers:
satorux1, kinaba
CC:
chromium-reviews, nkostylev+watch_chromium.org, yoshiki+watch_chromium.org, rginda+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, chrome-apps-syd-reviews_chromium.org
Project:
chromium
Visibility:
Public.

Description

[Files App] Only use the first matching file handler from each app. When an app has multiple file handlers for the same file type, it is shown twice in the "Open With" menu. This CL ensures that only one file handler is taken from each app. TEST=Install an app with file handlers (e.g. Text). Open the Files app and select a handled file (e.g. .txt). The "Open" menu on the bottom right (next to the trash can) should have a drop-down button. Click the drop-down, it should list Text. Select the file again and right-click it. Click "Open With". A list of apps should pop up that includes Text. BUG=350193 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287493

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -19 lines) Patch
M chrome/browser/chromeos/file_manager/file_tasks.cc View 1 chunk +19 lines, -19 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
jackhou1
satorux, could you please review this?
6 years, 4 months ago (2014-08-04 07:06:49 UTC) #1
satorux1
looks good to me but defer to kinaba@ just in case.
6 years, 4 months ago (2014-08-04 07:21:38 UTC) #2
satorux1
Could you also add a TEST= line and adds instructions on how to test that ...
6 years, 4 months ago (2014-08-04 07:22:49 UTC) #3
kinaba
On 2014/08/04 07:21:38, satorux1 wrote: > looks good to me but defer to kinaba@ just ...
6 years, 4 months ago (2014-08-04 21:40:22 UTC) #4
jackhou1
The CQ bit was checked by jackhou@chromium.org
6 years, 4 months ago (2014-08-05 01:34:44 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jackhou@chromium.org/415403006/1
6 years, 4 months ago (2014-08-05 01:35:48 UTC) #6
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_chromium_gn_compile_rel on tryserver.chromium.linux ...
6 years, 4 months ago (2014-08-05 02:27:01 UTC) #7
commit-bot: I haz the power
6 years, 4 months ago (2014-08-05 08:13:05 UTC) #8
Message was sent while issue was closed.
Change committed as 287493

Powered by Google App Engine
This is Rietveld 408576698