New issue
Advanced search Search tips

Issue 786208 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

[Media Router UI] Cleanup work regarding route descriptions

Project Member Reported by mfo...@chromium.org, Nov 17 2017

Issue description

This is to track future Chrome-side cleanups to simplify how media routes are shown in the UI.  Search for TODO(crbug.com/XXX) where XXX is this bug number to find out what they are.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 18 2017

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

commit eac8592c0c9efc5fbe0e8271a2e88e84b35f914a
Author: mark a. foltz <mfoltz@chromium.org>
Date: Sat Nov 18 01:09:54 2017

[Media Router UI] Simplify rendering of route descriptions in the MR dialog.

We want to consolidate how route descriptions passed to the UX by using
MediaRoute.description instead of MediaStatus.description.  This adds comments
explaining what the route description should be used for, and notes that
MediaStatus.description will be deprecated once MRPs are updated to use
MediaRoute.description instead.

It also removes the "Casting" prefix which was sometimes inserted before
MediaRoute.description, as we want to customize the description to say
"Casting," "Remoting," or "Mirroring."

The template string remains so we can use it in a future change
for proper localization of the description.  This change does not make the UX
any worse than it is right now (and will be followed soon by an MRP changes to
add "Casting" back where it makes sense).

The Media Router elements are refactored to use "description" consistently and
to better share CSS, which makes rendering more consistent between
MediaStatus.description and MediaRoute.description.

TBR=estark@chromium.org for comment-only .mojom changes

Bug:  786208 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I9356fa917f0d77eeac5b7b32d6175aa03a943e5c
Reviewed-on: https://chromium-review.googlesource.com/776130
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517656}
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/browser/resources/media_router/elements/route_controls/route_controls.css
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/browser/resources/media_router/elements/route_controls/route_controls.html
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/browser/resources/media_router/elements/route_controls/route_controls.js
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/browser/resources/media_router/elements/route_details/route_details.css
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/browser/resources/media_router/elements/route_details/route_details.html
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/browser/resources/media_router/elements/route_details/route_details.js
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/browser/resources/media_router/media_router_common.css
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/common/media_router/media_route.h
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/common/media_router/media_status.h
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/common/media_router/mojo/media_router.mojom
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/common/media_router/mojo/media_status.mojom
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/test/data/webui/media_router/route_controls_tests.js
[modify] https://crrev.com/eac8592c0c9efc5fbe0e8271a2e88e84b35f914a/chrome/test/data/webui/media_router/route_details_tests.js

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 7 2018

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

commit 694126144e36b4645d178c1cca477a4d12820f2c
Author: mark a. foltz <mfoltz@chromium.org>
Date: Wed Mar 07 21:40:39 2018

[Media Router UI] Cleanups related to media status.

- Removes references to 'description' and 'status' in MediaStatus. Removal from
  the mojo interface will be done in a separate patch.

- Don't show media title if it is empty, undefined, or duplicates the route
  description.  This works around scenarios where the Cast media controls
  are sending duplicates.

- Fixes a bug: JS calls to set the max dialog height were being made
  before the WebUI was loaded.  This may a the cause of flakiness on the
  Media Router bots.

Bug:  786208 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I2df6f28be0644ae3815f8dadee3cf53b2deeddd2
Reviewed-on: https://chromium-review.googlesource.com/949448
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541588}
[modify] https://crrev.com/694126144e36b4645d178c1cca477a4d12820f2c/chrome/browser/resources/media_router/elements/route_controls/route_controls.html
[modify] https://crrev.com/694126144e36b4645d178c1cca477a4d12820f2c/chrome/browser/resources/media_router/elements/route_controls/route_controls.js
[modify] https://crrev.com/694126144e36b4645d178c1cca477a4d12820f2c/chrome/browser/resources/media_router/elements/route_details/route_details.html
[modify] https://crrev.com/694126144e36b4645d178c1cca477a4d12820f2c/chrome/browser/resources/media_router/media_router_data.js
[modify] https://crrev.com/694126144e36b4645d178c1cca477a4d12820f2c/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/694126144e36b4645d178c1cca477a4d12820f2c/chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc
[modify] https://crrev.com/694126144e36b4645d178c1cca477a4d12820f2c/chrome/browser/ui/webui/media_router/media_router_webui_message_handler_unittest.cc
[modify] https://crrev.com/694126144e36b4645d178c1cca477a4d12820f2c/chrome/test/data/webui/media_router/route_controls_tests.js

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 9 2018

Status: Fixed (was: Assigned)
There's no more work to do around this on the browser side.  Remaining fixes/cleanups are in the component extension.  Closing.

Comment 5 by mfo...@chromium.org, Mar 31 2018

Labels: -Hotlist-Fixit-PE2017

Sign in to add a comment