New issue
Advanced search Search tips

Issue 590850 link

Starred by 1 user

Issue metadata

Status: Archived
Owner: ----
Closed: Sep 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Team-Security-UX



Sign in to add a comment

MenuButtons used instead of Combobox

Project Member Reported by est...@chromium.org, Feb 29 2016

Issue description

I don't think there's a good reason why we use MenuButton for 

- ContentSettingBubbleContents's |menu_button|s
- PermissionMenuButton
- PermissionCombobox (despite its name!)

by using a MenuButton we
(a) miss out on normal accelerator and event handling
(b) have to reimplement basic functionality such as PermissionCombobox::GetAccessibleState
(c) get random bugs like  bug 590171 

+felt to triage
 

Comment 1 by est...@chromium.org, Feb 29 2016

oh yea, plus 

(d) visually it looks weird/wrong when the options are showing
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 29 2016

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

commit 923d19c7b8dad55f35d0f2aeaae5975f0b002930
Author: estade <estade@chromium.org>
Date: Mon Feb 29 23:24:08 2016

Views - Fix text elision in content settings bubble's faux combobox

BUG= 590171 , 590850 

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

Cr-Commit-Position: refs/heads/master@{#378314}

[modify] https://crrev.com/923d19c7b8dad55f35d0f2aeaae5975f0b002930/chrome/browser/ui/views/content_setting_bubble_contents.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 3 2016

Labels: merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6761aebd1bb77c21e9fabf78cb134a65aa65b49a

commit 6761aebd1bb77c21e9fabf78cb134a65aa65b49a
Author: Evan Stade <estade@chromium.org>
Date: Thu Mar 03 21:44:07 2016

Views - Fix text elision in content settings bubble's faux combobox

BUG= 590171 , 590850 

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

Cr-Commit-Position: refs/heads/master@{#378314}
(cherry picked from commit 923d19c7b8dad55f35d0f2aeaae5975f0b002930)

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

Cr-Commit-Position: refs/branch-heads/2661@{#73}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/6761aebd1bb77c21e9fabf78cb134a65aa65b49a/chrome/browser/ui/views/content_setting_bubble_contents.cc

Comment 4 by f...@chromium.org, Mar 16 2016

Owner: est...@chromium.org
Status: Fixed (was: Assigned)
thanks for fixing, marking you as the owner & the bug as fixed?

Comment 5 by est...@chromium.org, Mar 16 2016

Owner: ----
Status: Available (was: Fixed)
no, I didn't fix it. I band-aided.

Comment 7 by tapted@chromium.org, May 25 2016

Cc: palmer@chromium.org tapted@chromium.org ellyjo...@chromium.org
Elly's actually started exploring doing this for PermissionMenuButton in https://codereview.chromium.org/2011963002/ - but initially just for Mac. Perhaps we should just opt in to combobox for all platforms.

Comment 8 by est...@chromium.org, May 26 2016

yes indeed
Components: UI>Browser>Bubbles>PageInfo
Components: -UI>Browser>Permissions
Labels: Hotlist-EnamelAndFriendsFixIt
Labels: -Hotlist-EnamelAndFriendsFixIt
Status: Archived (was: Available)
Archiving old bugs that haven't been actively assigned in over 180 days.

If you feel this issue should still be addressed, feel free to reopen it or to file a new issue. Thanks!
Archiving old bugs that haven't been actively assigned in over 180 days.

If you feel this issue should still be addressed, feel free to reopen it or to file a new issue. Thanks!
Archiving old bugs that haven't been actively assigned in over 180 days.

If you feel this issue should still be addressed, feel free to reopen it or to file a new issue. Thanks!
Archiving old bugs that haven't been actively assigned in over 180 days.

If you feel this issue should still be addressed, feel free to reopen it or to file a new issue. Thanks!
Archiving old bugs that haven't been actively assigned in over 180 days.

If you feel this issue should still be addressed, feel free to reopen it or to file a new issue. Thanks!

Sign in to add a comment