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

Issue 867389 link

Starred by 1 user
Status: Fixed
Owner:
beccahughes@chromium.org
Closed: Jul 31
Cc:
█ manoranj...@chromium.org
nyerramilli@chromium.org
█ apaci...@chromium.org
rbasuvula@chromium.org
alex...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression:Close('x') button is seen on LHS top and it works when clicked on RHS top.

Reported by shruti.j...@etouch.net, Jul 25 
Chrome Version:69.0.3497.12 (Official Build) Revision	e8e14b69c41a5a461c15fa5d1065ec28c49890ac-refs/branch-heads/3497@{#54}(64-bit)

OS: Windows(7,8,8.1,10)and Mac(10.12.6, 10.13.1, 10.13.6, 10.14).

Pre-condition:Enable 'RTL' from chrome://flags
Test URL:https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html

Steps to reproduce:
1. Launch chrome, navigate to above test URL and switch to 'Picture in Picture' mode from three-dot menu on video.
2. Click on Close('x') button present on 'Picture in Picture' mode.
3. Observe Close('x') button.

Actual Result: Close('x') button is seen on LHS top and it works when clicked on RHS top.
Expected Result:Close('x') button  should be seen on RHS. 

This is a regression issue, broken in 'M-69', and below is  bisect-info:
Good Build:69.0.3489.0(Revision:574446)
Bad Build: 69.0.3491.0(Revision:575129)

Narrow Bisect info : 
https://chromium.googlesource.com/chromium/src/+log/c698a3600cde3a3f002ec5248c50c49c7cfa5fc7..a0028fe6ee13c31ab280529bdc30fa2e2e74eecd

Suspect: https://chromium.googlesource.com/chromium/src/+/94a981bb9e1fedb98b268ffae946073bf266c6ca

@Becca Hughes: 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:As Becca Hughes is OOO keeping  'apacible@chromium.org, alexmos@chromium.org' in CC.

Thank You.
Actual_Result.mp4
696 KB View Download
Expected_Result.mp4
610 KB View Download

Comment 1 by nyerramilli@chromium.org, Jul 25

Cc: manoranj...@chromium.org
Labels: ReleaseBlock-Stable
marking as RBS, please change if required.

Comment 2 by beccahughes@chromium.org, Jul 27

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 30 

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

commit 64a3d2da689ff2ce03ec89332a9b2e1d663a2e08
Author: Becca Hughes <beccahughes@chromium.org>
Date: Mon Jul 30 16:57:49 2018

[Picture in Picture] Fix button click in RTL mode

Fix button click in RTL mode by changing GetCloseControlsBounds()
to use GetMirroredBounds(). Also removes the OnMouseEvent button
handlers as Button will do this.

BUG= 867389 

Change-Id: Iaeb8a8059593592da2ad59c57cf0d98b128a47ca
Reviewed-on: https://chromium-review.googlesource.com/1153783
Reviewed-by: apacible <apacible@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579072}
[modify] https://crrev.com/64a3d2da689ff2ce03ec89332a9b2e1d663a2e08/chrome/browser/ui/views/overlay/overlay_window_views.cc

Comment 4 by beccahughes@chromium.org, Jul 30

Labels: Merge-Request-69

Comment 5 by shruti.j...@etouch.net, Jul 31

Labels: TE-Verified-70.0.3508.0 TE-Verified-M70
Update : 
Retested above issue on Windows (7,8,8.1,10) and Mac(10.12.6, 10.13.1, 10.13.6, 10.14) OS using latest Canary #70.0.3508.0 and issue is fixed.CLose button now works fine on RTL .Kindly review the attached screen-cast.
Thank you
Canary_Behaviour#70.0.3508.0..mp4
674 KB View Download

Comment 6 by gov...@chromium.org, Jul 31

Labels: -Merge-Request-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #5, pls merge now. Thank you.
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 31

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1069c55f3b7f0f836b1143b83b66176d4c2404e

commit b1069c55f3b7f0f836b1143b83b66176d4c2404e
Author: Becca Hughes <beccahughes@chromium.org>
Date: Tue Jul 31 17:50:28 2018

[Picture in Picture] Fix button click in RTL mode

Fix button click in RTL mode by changing GetCloseControlsBounds()
to use GetMirroredBounds(). Also removes the OnMouseEvent button
handlers as Button will do this.

BUG= 867389 

Change-Id: Iaeb8a8059593592da2ad59c57cf0d98b128a47ca
Reviewed-on: https://chromium-review.googlesource.com/1153783
Reviewed-by: apacible <apacible@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579072}(cherry picked from commit 64a3d2da689ff2ce03ec89332a9b2e1d663a2e08)
Reviewed-on: https://chromium-review.googlesource.com/1156931
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#286}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/b1069c55f3b7f0f836b1143b83b66176d4c2404e/chrome/browser/ui/views/overlay/overlay_window_views.cc

Comment 8 by beccahughes@chromium.org, Jul 31

Status: Fixed (was: Started)

Comment 9 by shruti.j...@etouch.net, Aug 1

Labels: TE-Verified-M69 TE-Verified-69.0.3497.23
Update : 
Retested above issue on Windows (7,8,8.1,10) and Mac(10.12.6, 10.13.1, 10.13.6, 10.14) OS using Dev #69.0.3497.23and issue is fixed.Close button now works fine on RTL .Kindly review the attached screen-cast.
Thank you
Dev#69.0.3497.23.mov
3.4 MB View Download
►

Sign in to add a comment