Regression: Google icon (Super G icon) appears inverted in Omnibox
Reported by
khushal....@etouch.net,
Sep 7
|
||||||||||||||
Issue descriptionChrome Version: 71.0.3545.0 (Official Build) Revision 38ca181b5b0849226f5f15de263301b40cd36b5d-refs/branch-heads/3545@{#1} (32/64-bit) OS: Windows(7,8,8.1,10), Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14) and Linux(14.04 LTS) Pre-condition: Enable flag "Force UI direction" from chrome://flags. Steps to reproduce: 1. Launch chrome and navigate to Google.com 2. Now open new NTP and observe Google icon(Super G icon). Actual Result : Google icon(Super G icon) appears inverted in Omnibox. Expected Result: Google icon(Super G icon) should be seen properly. This is a regression issue broken in 'M-70' and will soon update the bisect info: Good build: 70.0.3537.0 (Revision:587303) Bad build : 70.0.3538.0 (Revision:587811) Thank You.
,
Sep 7
,
Sep 7
,
Sep 7
[omnibox triage: ReleaseBlock-Stable, as this is highly visible, even if it only effects RTL users assigning, as Pri-1 shoulds be assigned; feel free to reassign to someone else if you want]
,
Sep 10
Update: (Unable to provide bisect using 1) Per-revision script as "[Errno 2] No such file or directory" error message is thrown and 2) Chromium bisect script as Getting unrelated change log, Also, tried bisecting on multiple machines and same error is thrown. Hence, providing suspect manually from change-log) CHANGE-LOG URL: https://chromium.googlesource.com/chromium/src/+log/70.0.3537.0..70.0.3538.0?pretty=fuller&n=10000 Suspecting: https://chromium.googlesource.com/chromium/src/+/08acb8994f55b1c1ff31ef5820a86ba18d25ec09 Thank You..!!
,
Sep 17
Friendly ping! Could you please provide any update on this issue as it has been marked as a stable blocker. Thank You!
,
Sep 17
95% sure this is the CL that regressed this behavior. https://chromium.googlesource.com/chromium/src/+/c8416d4817f64e0a51d14455346d1d011a951c16
,
Sep 19
Checked with the previous commit, and it looks like it indeed regressed on my CL. I'll see if the fix is merge-able for 70, otherwise I'll revert the CL, and disable animations on the security chip to fix the bug that CL fixed.
,
Sep 27
Sent fix for review in crrev.com/c/1250055
,
Sep 27
,
Oct 1
[bulk edit] - This issue is marked as a stable blocker for M70. We are two weeks away from M70 Stable. Please take a look urgently!
,
Oct 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4fd375c58e6c06f27d574720b6a815baee450ad7 commit 4fd375c58e6c06f27d574720b6a815baee450ad7 Author: Carlos IL <carlosil@chromium.org> Date: Mon Oct 01 20:03:25 2018 Removed EnableFlippingForRTLUI from SetUpForInOutAnimation. SetupForInOutAnimation (in IconLabelBubbleView) sets 'EnableFlippingFoRTLUI' to true in all cases. Since only ContentSettingImageView needs it set to true, removed it from the method and it is now set separately in that subclass Bug: 881798 Change-Id: I2d570cc84d9d4f9443b39ffa51d943d938624dbe Reviewed-on: https://chromium-review.googlesource.com/1250055 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Carlos IL <carlosil@chromium.org> Cr-Commit-Position: refs/heads/master@{#595528} [modify] https://crrev.com/4fd375c58e6c06f27d574720b6a815baee450ad7/chrome/browser/ui/views/location_bar/content_setting_image_view.cc [modify] https://crrev.com/4fd375c58e6c06f27d574720b6a815baee450ad7/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc [modify] https://crrev.com/4fd375c58e6c06f27d574720b6a815baee450ad7/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h
,
Oct 1
Fix just landed, will request merge approval to 70 once it makes it to Canary.
,
Oct 2
This is now in canary, requesting merge approval.
,
Oct 2
This bug requires manual review: We are only 13 days from stable. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 2
,
Oct 2
Branch:3538
,
Oct 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0fa87649deee4dd61479f76b0aecd990ae83d60 commit f0fa87649deee4dd61479f76b0aecd990ae83d60 Author: Carlos IL <carlosil@chromium.org> Date: Tue Oct 02 16:55:24 2018 Removed EnableFlippingForRTLUI from SetUpForInOutAnimation. SetupForInOutAnimation (in IconLabelBubbleView) sets 'EnableFlippingFoRTLUI' to true in all cases. Since only ContentSettingImageView needs it set to true, removed it from the method and it is now set separately in that subclass Bug: 881798 Change-Id: I2d570cc84d9d4f9443b39ffa51d943d938624dbe Reviewed-on: https://chromium-review.googlesource.com/1250055 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Carlos IL <carlosil@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#595528}(cherry picked from commit 4fd375c58e6c06f27d574720b6a815baee450ad7) Reviewed-on: https://chromium-review.googlesource.com/1257363 Reviewed-by: Carlos IL <carlosil@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#818} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/f0fa87649deee4dd61479f76b0aecd990ae83d60/chrome/browser/ui/views/location_bar/content_setting_image_view.cc [modify] https://crrev.com/f0fa87649deee4dd61479f76b0aecd990ae83d60/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc [modify] https://crrev.com/f0fa87649deee4dd61479f76b0aecd990ae83d60/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h
,
Oct 2
,
Oct 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0fa87649deee4dd61479f76b0aecd990ae83d60 Commit: f0fa87649deee4dd61479f76b0aecd990ae83d60 Author: carlosil@chromium.org Commiter: carlosil@chromium.org Date: 2018-10-02 16:55:24 +0000 UTC Removed EnableFlippingForRTLUI from SetUpForInOutAnimation. SetupForInOutAnimation (in IconLabelBubbleView) sets 'EnableFlippingFoRTLUI' to true in all cases. Since only ContentSettingImageView needs it set to true, removed it from the method and it is now set separately in that subclass Bug: 881798 Change-Id: I2d570cc84d9d4f9443b39ffa51d943d938624dbe Reviewed-on: https://chromium-review.googlesource.com/1250055 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Carlos IL <carlosil@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#595528}(cherry picked from commit 4fd375c58e6c06f27d574720b6a815baee450ad7) Reviewed-on: https://chromium-review.googlesource.com/1257363 Reviewed-by: Carlos IL <carlosil@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#818} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
,
Oct 3
Update: Rechecked the above issue on Windows (7, 8, 8.1, 10), Mac (10.12.6, 10.13.1, 10.13.6, 10.14.1) and Linux (14.04 LTS) using Beta version #70.0.3538.45 and the issue is found FIXED. Hence, adding TE verified labels. Please refer the attached screen-cast. Thank You..!! |
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by rbasuvula@chromium.org
, Sep 7Status: Untriaged (was: Unconfirmed)