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

Issue 612969 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Media Router WebUI search feature should not be enabled by default.

Project Member Reported by btolsch@chromium.org, May 18 2016

Issue description

The Media Router dialog search feature should only be enabled if the sink list is long enough to make filtering useful or if search is available (e.g. if cloud is enabled).
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 20 2016

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

commit 4d46c6973164274ba1e67242eae30bbc9050fce0
Author: btolsch <btolsch@chromium.org>
Date: Fri May 20 19:06:57 2016

[Media Router WebUI] Disable search by default.

This change disables the search feature by default, only enabling it for
exceptionally long sink lists and when pseudo sinks are available.
Whether search is enabled is determined when the list of available sinks
is updated. Once it has been enabled, it will not be disabled for the
rest of the life of the dialog.

R=amp@chromium.org, apacible@chromium.org
BUG= 612969 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/1993003002
Cr-Commit-Position: refs/heads/master@{#395128}

[modify] https://crrev.com/4d46c6973164274ba1e67242eae30bbc9050fce0/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.css
[modify] https://crrev.com/4d46c6973164274ba1e67242eae30bbc9050fce0/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.html
[modify] https://crrev.com/4d46c6973164274ba1e67242eae30bbc9050fce0/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
[modify] https://crrev.com/4d46c6973164274ba1e67242eae30bbc9050fce0/chrome/browser/resources/media_router/media_router_data.js
[modify] https://crrev.com/4d46c6973164274ba1e67242eae30bbc9050fce0/chrome/test/data/webui/media_router/media_router_container_filter_tests.js
[modify] https://crrev.com/4d46c6973164274ba1e67242eae30bbc9050fce0/chrome/test/data/webui/media_router/media_router_container_search_tests.js

Status: Fixed (was: Started)
Labels: Merge-Request-51

Comment 4 by tin...@google.com, May 21 2016

Labels: -Merge-Request-51 Merge-Review-51 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M51, manual review required.

Comment 5 by gov...@chromium.org, May 23 2016

Before we approve merge to M51, Could you please confirm whether this change is baked/verified in Canary and safe to merge?
Yes, this is in Canary and safe to merge.

Comment 7 by gov...@chromium.org, May 23 2016

Cc: sshruthi@chromium.org
Labels: -Merge-Review-51 Merge-Approved-51
Thank you, approving merge to M51 branch 2704 based on comment #6. Please merge before 5:00 PM PST today (Monday) so we can take it for M51 Desktop Stable candidate cut.
Labels: Merge-Request-52

Comment 9 by tin...@google.com, May 23 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 10 by bugdroid1@chromium.org, May 23 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1934ec78f57bbc3a257fdfd12bf6282edf129e6a

commit 1934ec78f57bbc3a257fdfd12bf6282edf129e6a
Author: btolsch <btolsch@chromium.org>
Date: Mon May 23 20:32:57 2016

[Media Router WebUI] Disable search by default.

This change disables the search feature by default, only enabling it for
exceptionally long sink lists and when pseudo sinks are available.
Whether search is enabled is determined when the list of available sinks
is updated. Once it has been enabled, it will not be disabled for the
rest of the life of the dialog.

R=amp@chromium.org, apacible@chromium.org
BUG= 612969 
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/1993003002
Cr-Commit-Position: refs/heads/master@{#395128}
(cherry picked from commit 4d46c6973164274ba1e67242eae30bbc9050fce0)

Review-Url: https://codereview.chromium.org/2004133002
Cr-Commit-Position: refs/branch-heads/2743@{#19}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/1934ec78f57bbc3a257fdfd12bf6282edf129e6a/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.css
[modify] https://crrev.com/1934ec78f57bbc3a257fdfd12bf6282edf129e6a/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.html
[modify] https://crrev.com/1934ec78f57bbc3a257fdfd12bf6282edf129e6a/chrome/browser/resources/media_router/elements/media_router_container/media_router_container.js
[modify] https://crrev.com/1934ec78f57bbc3a257fdfd12bf6282edf129e6a/chrome/browser/resources/media_router/media_router_data.js
[modify] https://crrev.com/1934ec78f57bbc3a257fdfd12bf6282edf129e6a/chrome/test/data/webui/media_router/media_router_container_filter_tests.js
[modify] https://crrev.com/1934ec78f57bbc3a257fdfd12bf6282edf129e6a/chrome/test/data/webui/media_router/media_router_container_search_tests.js

Labels: -Merge-Approved-51
We won't be merging this into M51 after all due to some i18n changes.

Sign in to add a comment