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

Issue 613253 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
no longer active
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

[Media Router] Issues from extension don't show multiple actions.

Project Member Reported by amp@chromium.org, May 19 2016

Issue description

Version: 52.0.2742.0 (built locally from HEAD)
OS: Saw on Linux, but probably affects all

What steps will reproduce the problem?
(1) send an issue to MR (from background page of MR extension)
var issue = new mr.Issue('title', mr.IssueSeverity.NOTIFICATION, mr.IssueAction.LEARN_MORE);
issue.setSecondaryActions([mr.IssueAction.DISMISS]); issue.setHelpUrl("https://example.com");
mr.Init.providerManager_.getIssueSender().send(issue);
(2) Observe issue displayed in MR UI

What is the expected output?
Issue should show two actions (LEARN MORE and DISMISS).


What do you see instead?
https://screenshot.googleplex.com/W6xyHUnWbTU
And clicking on 'LEARN MORE' closes the issue without opening the url.


Somewhat related but possible a different issue, it appears that the bindings refer to enums that were never created, see the mojom enums:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/media/router/mojo/media_router.mojom&l=74
and the bindings referring to them:
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/renderer/resources/media_router_bindings.js&l=330

 
Cc: -apaci...@chromium.org
Labels: -OS-All OS-Chrome OS-Linux OS-Mac OS-Windows
Owner: apaci...@chromium.org
Status: Started (was: Untriaged)
Summary: [Media Router] Issues from extension don't show multiple actions. (was: [Media Router] Issue API not working (helpUrl's doesn't link and doesn't show multiple actions))
Renaming as we're tracking the help URLs not opening in  crbug.com/613242 
Project Member

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

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

commit 8d0894e8a5b6c29e97704a62ca75cc1cb412223d
Author: apacible <apacible@chromium.org>
Date: Mon May 23 19:59:07 2016

[Media Router WebUI] Show issue's secondary action when 'dismiss'.

The action type 'dismiss' maps to 0. Currently, we don't check whether it's undefined, but if it is truthy. if(0) is always false. This means that if the secondary button is 'dismiss', it will not be shown.

This was previously not caught as the tests mark non-existent secondary actions as null rather than undefined.

BUG= 613253 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/8d0894e8a5b6c29e97704a62ca75cc1cb412223d/chrome/browser/resources/media_router/elements/issue_banner/issue_banner.js
[modify] https://crrev.com/8d0894e8a5b6c29e97704a62ca75cc1cb412223d/chrome/browser/resources/media_router/media_router_data.js
[modify] https://crrev.com/8d0894e8a5b6c29e97704a62ca75cc1cb412223d/chrome/test/data/webui/media_router/issue_banner_tests.js

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

Comment 5 by tin...@google.com, May 25 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 6 by bugdroid1@chromium.org, May 25 2016

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

commit 802c756a74fcabb788563982e2ba48a152132019
Author: Jennifer Apacible <apacible@google.com>
Date: Wed May 25 18:27:40 2016

[Media Router WebUI] Show issue's secondary action when 'dismiss'.

The action type 'dismiss' maps to 0. Currently, we don't check whether it's undefined, but if it is truthy. if(0) is always false. This means that if the secondary button is 'dismiss', it will not be shown.

This was previously not caught as the tests mark non-existent secondary actions as null rather than undefined.

BUG= 613253 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/1998013003
Cr-Commit-Position: refs/heads/master@{#395394}
(cherry picked from commit 8d0894e8a5b6c29e97704a62ca75cc1cb412223d)

Review URL: https://codereview.chromium.org/2014673002 .

Cr-Commit-Position: refs/branch-heads/2743@{#58}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/802c756a74fcabb788563982e2ba48a152132019/chrome/browser/resources/media_router/elements/issue_banner/issue_banner.js
[modify] https://crrev.com/802c756a74fcabb788563982e2ba48a152132019/chrome/browser/resources/media_router/media_router_data.js
[modify] https://crrev.com/802c756a74fcabb788563982e2ba48a152132019/chrome/test/data/webui/media_router/issue_banner_tests.js

Status: Verified (was: Fixed)
Verified with 53.0.2762.0 (though note the setHelpUrl step is replaced with setHelpPageId).

Sign in to add a comment