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

Issue 669870 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression
Team-Security-UX



Sign in to add a comment

Regression : In omnibox, the focus on ‘View site info’ icon is not seen properly i.e the right edge of focus is seen chopped.

Reported by yfulgaon...@etouch.net, Nov 30 2016

Issue description

Chrome Version : 57.0.2937.0 276217cd97923c96f30654e6f459d23983855603-refs/heads/master@{#435110} 64 bit
OS : Mac(10.11.6, 10.12.1, 10.12)

What steps will reproduce the problem?
1. Launch chrome and navigate to chrome://settings page.
2. Click on ‘View site info’ icon in omnibox and observe the right edge of focus.

Actual : The focus on ‘View site info’ icon is not seen properly i.e the right edge of focus is seen chopped.
Expected : The focus on ‘View site info’ icon should be seen properly.

This is a regression issue broken in ‘M-57’, below is the Manual Regression range and will soon update other info.
Good build : 57.0.2936.0
Bad build : 57.0.2937.0

Note : This is Mac specific issue and the same is not reproducible on Windows & Linux OS.
 
Actual_focus.png
47.0 KB View Download
Actual_focus_1.mov
5.0 MB Download
Labels: hasbisect-per-revision
Owner: spqc...@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build: 57.0.2936.0 (Revision: 434845).
Bad build: 57.0.2937.0 (Revision: 435110).

You are probably looking for a change made after 434928 (known good), but no later than 434929 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/c89bbe3f914566c01af8509e05bf069236e3166e..c3a5825e0723184c3f72bffdc916035249318390

@spqchan -- Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.
Adding RB Label as this is a recent Regression. Please remove if not required.

Thank You.
Labels: ReleaseBlock-Stable

Comment 3 by meh...@chromium.org, Nov 30 2016

Looks like a dupe of  issue 669568 .
Gentle ping!!

Still we are able to reproduce the issue on mac 10.1.6.

@spqchan-Could you please check & update us on the same as this is marked as Release Block label.

Thank you.
Cc: spqc...@chromium.org shrike@chromium.org
 Issue 669568  has been merged into this issue.
Status: Started (was: Assigned)
I've been playing around with this. I tried to remove the extra white padding on the left of the NSTextfield but have no luck. Online research suggested that the only way to remove it is via custom drawing or hacks.

I've been using Hopper to see if there's anything I can hack for it. If I can't hack it, I'll custom draw it. Worst case, I'll see if it's okay for me to move it 1pt to the right, and then also move the omnibox dialog text 1pt to the right.
I think it shouldn't be difficult to hack around this problem. As I understand it, some method in NSTextField (or more likely NSTextFieldCell?) is drawing white pixels on the left side of the textfield. So you could do the following:

- (void)drawWithFrame:(NSRect)frame inView:(NSView*)controlView {
  NSRect clipRect = frame;
  clipRect.origin.x += 1;
  clipRect.size.width -= 1;
  [NSBezierPath clipRect:clipRect];

  [super drawWithFrame:frame inView:controlView];
}

This code basically prevents anyone from drawing anything along the textfield's left edge.
Owner: shrike@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 9 2016

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

commit 55c13aa9610ea5e5dc91f3d3b21c5cc42d05ff5c
Author: shrike <shrike@chromium.org>
Date: Fri Dec 09 18:46:57 2016

[Mac] Prevent omnibox text bg from drawing over  decoration, focus ring.

The AutocompleteTextFieldEditor draws a background, apparently even when
you call setDrawsBackground:NO (at least during editing). Because of the
1pt spacing set by UX between the security decoration border and the
start of omnibox text, this background started drawing into the
decoration's hover rect. On the right edge it drew into the edge of the
focus ring.

This cl	prevents the AutocompleteTetFieldEditor	from drawing a
background, relying instead on the background drawn by the textfield's
cell. It also changes decorations to be drawn after the cell's interior,
to prevent the background overlap when not editing.

R=avi@chromium.org
BUG= 669870 , 672518 

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

[modify] https://crrev.com/55c13aa9610ea5e5dc91f3d3b21c5cc42d05ff5c/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm
[modify] https://crrev.com/55c13aa9610ea5e5dc91f3d3b21c5cc42d05ff5c/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm

This broken in M-56. Changing the Mstone-label.
Labels: -M-57 M-56
Labels: TE-Verified-M57 TE-Verified-57.0.2949.0
Tested the same on mac 10.11.6 chrome version 57.0.2949.0 -  The focus on ‘View site info’ icon displayed fine

Please find the screenshot
Screen Shot 2016-12-12 at 5.22.11 PM.png
184 KB View Download
Labels: Merge-Request-56

Comment 14 by dimu@chromium.org, Dec 12 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Your change has been approved for M56. Please merge your CL ASAP so that we could take it for next Beta and RC at 4.00 PM today- Dec 13.
Project Member

Comment 16 by sheriffbot@chromium.org, Dec 16 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 17 by sheriffbot@chromium.org, Dec 19 2016

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
shrike@, Your change has been approved for M56. Please merge your fix to M56
Project Member

Comment 19 by bugdroid1@chromium.org, Jan 4 2017

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb61cdb18135ca7d69f2431a2af15ccced4b4e1b

commit bb61cdb18135ca7d69f2431a2af15ccced4b4e1b
Author: Jayson Adams <shrike@chromium.org>
Date: Wed Jan 04 00:26:56 2017

[Mac] Prevent omnibox text bg from drawing over  decoration, focus ring.

The AutocompleteTextFieldEditor draws a background, apparently even when
you call setDrawsBackground:NO (at least during editing). Because of the
1pt spacing set by UX between the security decoration border and the
start of omnibox text, this background started drawing into the
decoration's hover rect. On the right edge it drew into the edge of the
focus ring.

This cl	prevents the AutocompleteTetFieldEditor	from drawing a
background, relying instead on the background drawn by the textfield's
cell. It also changes decorations to be drawn after the cell's interior,
to prevent the background overlap when not editing.

R=avi@chromium.org
BUG= 669870 , 672518 

Review-Url: https://codereview.chromium.org/2562863002
Cr-Commit-Position: refs/heads/master@{#437596}
(cherry picked from commit 55c13aa9610ea5e5dc91f3d3b21c5cc42d05ff5c)

Review-Url: https://codereview.chromium.org/2610033003 .
Cr-Commit-Position: refs/branch-heads/2924@{#662}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/bb61cdb18135ca7d69f2431a2af15ccced4b4e1b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm
[modify] https://crrev.com/bb61cdb18135ca7d69f2431a2af15ccced4b4e1b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm

Cc: rbasuvula@chromium.org
Labels: TE-Verified-M56 TE-Verified-56.0.2924.51
Tested the same on mac 10.12.2 chrome version 56.0.2924.51 -  The focus on ‘View site info’ icon displayed fine

Please find the screenshot

Thank you!
669870.png
104 KB View Download
shrike@ - The fix is working as expected in latest beta #56.0.2924.51 as per comment #20.

Could you please change the status of this issue as fixed.

Thanks...!!
Status: Fixed (was: Started)

Sign in to add a comment