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

Issue 747782 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Unwanted white background is seen for installation prompt on chrome://apps.

Reported by aiman.an...@etouch.net, Jul 24 2017

Issue description

Chrome Version: 62.0.3165.0 (Official Build) ebd65246aab3b4b1cb5bd462388344195420078d-refs/heads/master@{#488876}(64-bit)

OS:  Mac(10.12.3, 10.11.6).

Test URL: https://chrome.google.com/webstore/detail/postman/fhbjgbiflinjbdggehcddcbncdddomop?utm_source=chrome-ntp-icon

Steps to reproduce:
1)Launch Chrome, go to the above URL, Click on ‘Add to Chrome Button’.
2)Observe the installation prompt.

Actual Result: Unwanted White background is seen for installation prompt.
Expected Result: White background should not be seen.

This is Regression issue broken in M-62 and will soon update bisect info.

Manual Bisect Info:
Good Build: 61.0.3163.0
Bad Build:  62.0.3164.0
 
Actual Result.mov
2.0 MB Download
Expected Result.mov
1.8 MB Download
Cc: jmukthavaram@chromium.org
Labels: -M-61 hasbisect-per-revision M-62
Owner: ellyjo...@chromium.org
Status: Assigned (was: Unconfirmed)
Able to reproduce the issue on Mac 10.12.5 using chrome latest Canary-62.0.3164.0.
Manual Bisect Info:
------------------
Good Build: 61.0.3163.0-Revision-488528
Bad Build:  62.0.3164.0-Revision-488823

Per revision bisect info:
-------------------------
You are probably looking for a change made after 488747 (known good), but no later than 488748 (first known bad).
CHANGELOG 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/0c5896f54772633e34c7fa4b7ec3450d93b8cc11..74b56fb13c1ebc11d1f2f08ec071379988a1a433

Possible suspect:
----------------
https://chromium.googlesource.com/chromium/src/+/74b56fb13c1ebc11d1f2f08ec071379988a1a433

ellyjones@Could you please take a look and reassign to the right owner if it is not related to your change.

Note: No issue observed on Windows 7 & Ubuntu 14.04

Thanks..!!
Cc: shrike@chromium.org
Yes, this was my change. Thanks for the report.

shrike: the Cocoa extension install dialog has historically had a white background until I fixed  issue 743120 , and one of its text areas has a white background, so it now mismatches the background color of the dialog. We can either:

1) Restore the white background to this dialog, or
2) Change the text area to not have an explicit background color

I prefer (2). I'll put a CL up shortly.

Comment 3 by shrike@chromium.org, Jul 27 2017

Hmm, I think I can go either way (1 or 2).

Comment 4 by tapted@chromium.org, Jul 27 2017

Note we've had trouble in the past with giving this text area a non-opaque background..

(extension_install_view_controller.mm has)

- (void)awakeFromNib {
  // Since linking to 10.10, |outlineView_| needs an explicit background to
  // ensure subpixel antialiasing is enabled for the permissions text. At the
  // same time, the animation that shows the prompt breaks whenever the scroll
  // view is present. Giving the scroll view a layer restores the animation, and
  // since its contents has an opaque background, subpixel AA isn't affected.
  [[outlineView_ enclosingScrollView] setWantsLayer:YES];
  [outlineView_ setBackgroundColor:[NSColor whiteColor]];


IMO we shouldn't change the extension install dialog background color without checking with Extensions/UX.
#4: I think we can set the background color to [NSColor windowBackgroundColor] like the code in <https://chromium-review.googlesource.com/c/575167/> did to avoid breaking the text AA code.

We can ask the UX folks about this change but it does not seem like they are suffused with free bandwidth for dealing with our vintage Cocoa UI, so if we can be reasonably confident about what the right way forward is, it might be better to just do that :)

Comment 6 by shrike@chromium.org, Jul 27 2017

Probably sticking with the old style (white background) would be safest.
Status: Started (was: Assigned)
https://chromium-review.googlesource.com/c/590491
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 31 2017

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

commit 567c089397ef001b5a2b645ddc31bfbc0c6e2e3a
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Mon Jul 31 13:48:13 2017

cocoa: return extension install dialog's white background

This dialog uses ConstrainedWindowCustomWindow, but it needs a white background
so that the contained text can be properly antialiased.

Bug:  747782 
Change-Id: I973b00096feaaea44ba72e699838f37ac8a3e518
Reviewed-on: https://chromium-review.googlesource.com/590491
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490756}
[modify] https://crrev.com/567c089397ef001b5a2b645ddc31bfbc0c6e2e3a/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm

Status: Fixed (was: Started)
Labels: TE-Verified-62.0.3174.0 TE-Verified-M62

Sign in to add a comment