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

Issue 782185 link

Starred by 1 user
Status: Fixed
Owner:
tapted@chromium.org
Closed: Nov 2017
Cc:
pbos@chromium.org
bsep@chromium.org
nyerramilli@chromium.org
tapted@chromium.org
rbasuvula@chromium.org
█ msrchandra@chromium.org
█ ranjitkan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Regression: Focus is lost on pressing tab key on password bubble.

Reported by aiman.an...@etouch.net, Nov 7 2017 
Chrome Version: 64.0.3261.0 (Official Build)aac97fe5f3cb5890bedac2a2ba9fec6b3e8c16fc-refs/heads/master@{#514329} (64-bit)

OS: Mac(10.12.6, 10.13.1).

Steps to reproduce:
1. Launch Chrome, login to Gmail with valid credentials
2. On password bubble press tab key and observe.

Actual Result: Focus is lost in between when focus travels from username and Show Password(eye) icon. (Also focus does not travel to cross icon).
Expected Result: Focus should travel properly in between username and Show Password(eye) icon.

This is regression issue broken in ‘M-64’ and below per-revision bisect result

Using the per-revision bisect providing the bisect results,
Good Build: 64.0.3260.0(Revision:514067)
Bad Build:64.0.3261.0(Revision:514329)

You are probably looking for a change made after 514116 (known good), but no later than 514117 (first known bad).

CHANGE-LOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/04f1082efb63369e19ace6dde8b62c8702303b88..6b6e8fe1fcf17e0545c688d1bfda16d6f79fd97e

Suspect: https://chromium.googlesource.com/chromium/src/+/6b6e8fe1fcf17e0545c688d1bfda16d6f79fd97e

@tapted: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Note: Issue is not seen on Linux and Win OS

Thank You!
Actual result.mov
7.7 MB Download
Expected Result.mov
7.7 MB Download

Comment 1 by tapted@chromium.org, Nov 7 2017

Cc: bsep@chromium.org
Labels: Proj-HarmonyDialogs
Does this behaviour match Windows? Seems like there's a view in the tab order that shouldn't be.

Focus is mean to skip the close [x], since it's considered part of the window frame.

Comment 2 by tapted@chromium.org, Nov 7 2017 

*Focus is meant to skip the close [x], since it's considered part of the window frame.

Comment 3 by aiman.an...@etouch.net, Nov 7 2017 

Hi,
No, On windows the focus travels properly.
Only on Mac the focus is lost while traveling from username to eye icon.

Comment 4 by bsep@chromium.org, Nov 7 2017

Cc: pbos@chromium.org
Labels: -Pri-1 -M-64 M-65 Pri-2
pbos@ FYI

Comment 5 by pbos@chromium.org, Nov 7 2017

Cc: tapted@chromium.org
Owner: pbos@chromium.org
Load balancing. tapted@ take it back if you were already looking into it.

Comment 6 by tapted@chromium.org, Nov 8 2017

Owner: tapted@chromium.org
Status: Started (was: Assigned)
heh - I bounced around a bit, but found the cause. In,

std::unique_ptr<views::Label> CreatePasswordLabel(
    const autofill::PasswordForm& form,
    bool is_password_visible) {
  base::string16 text =
      form.federation_origin.unique()
          ? form.password_value
          : l10n_util::GetStringFUTF16(
                IDS_PASSWORDS_VIA_FEDERATION,
                base::UTF8ToUTF16(form.federation_origin.host()));
  auto label = base::MakeUnique<views::Label>(text, CONTEXT_BODY_TEXT_LARGE,
                                              STYLE_SECONDARY);
  label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
  if (form.federation_origin.unique() && !is_password_visible)
    label->SetObscured(true);
  label->SetFocusBehavior(views::View::FocusBehavior::ACCESSIBLE_ONLY);
  return label;
}


we just need to delete that SetFocusBehavior line. It has no effect on non-Mac outside of an AccesiblePaneView, and it's not necessary on Mac to make views accessible to screen readers and other a11y clients.

Comment 7 by pbos@chromium.org, Nov 8 2017 

Cool, agreed on the root cause. Go ahead. :)
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 10 2017 

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

commit 4def1d7b841aa15a452930b95ce1df0e1080e875
Author: Trent Apted <tapted@chromium.org>
Date: Fri Nov 10 00:51:24 2017

MacViews Passwords bubble: Don't put the password label into the focus chain.

There's an unnecessary call to SetFocusBehavior on a views::Label.

Except on Mac, View::SetFocusBehavior(ACCESSIBLE_ONLY) only has meaning
inside a views::AccessiblePaneView. On Mac, it puts a view into the focus
chain when "Full Keyboard Access" is set in System Preferences. Screen
readers and a11y tools always traverse the full tree.

Bug:  782185 
Change-Id: I90402a05d3811c14704004d99d14f67c831fc648
Reviewed-on: https://chromium-review.googlesource.com/760077
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515384}
[modify] https://crrev.com/4def1d7b841aa15a452930b95ce1df0e1080e875/chrome/browser/ui/views/passwords/manage_password_items_view.cc

Comment 9 by tapted@chromium.org, Nov 10 2017

Status: Fixed (was: Started)

Comment 10 by aiman.an...@etouch.net, Nov 10 2017

Labels: TE-Verified-M64 TE-Verified-64.0.3264.0
Note:
Retested the above issue on latest Canary #64.0.3264.0 on Mac(10.12.6, 10.13.2) and fix is working as intended.
Current_Result.mov
6.1 MB Download
►

Sign in to add a comment