New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: ----


Participants' hotlists:
Launcher-A11y


Sign in to add a comment
link

Issue 913092: Shelf items go the wrong way when dragged in RTL.

Reported by newcomer@chromium.org, Dec 7 Project Member

Issue description

Chrome Version: M-72 branch ToT

What steps will reproduce the problem?
(1) Enable RTL language
(2) Drag a shelf app to the left

What is the expected result?
App goes left

What happens instead?
App goes right

This should be a super easy fix.

Video:
https://drive.google.com/file/d/13cir-3yZwBYZrmH8zFncyqfp2cjM65Sn/view?usp=sharing
 

Comment 1 by newcomer@chromium.org, Dec 7

(sorry for the potato quality video)

Comment 2 by manucornet@chromium.org, Dec 12

Didn't watch the video (not on corp network) but I can easily repro this... Wow, this behavior is very annoying :-)

Comment 3 by manucornet@chromium.org, Dec 12

Seems like this is related to this TODO:

// TODO: I don't think this works correctly with RTL.

https://cs.chromium.org/chromium/src/ash/shelf/shelf_view.cc?q=%22I+don%27t+think+this+works%22&sq=package:chromium&g=0&l=1265

This has been around since 2017 so may not be a regression, unless something else is going on.

Comment 4 by manucornet@chromium.org, Dec 12

Status: Started (was: Assigned)

Comment 5 by manucornet@chromium.org, Dec 12

FYI CL 1373452 sent out for review.

Comment 6 by manucornet@chromium.org, Dec 12

Summary: Shelf items go the wrong way when dragged in RTL. (was: Regression: Shelf items go the wrong way when dragged in RTL.)
Unclear to me that this is a regression given the fix. But if you have evidence that this worked in a previous build, glad to tweak the summary again of course.

Comment 7 by kaznacheev@chromium.org, Dec 12

Issue 804027 has been merged into this issue.

Comment 8 by manucornet@chromium.org, Dec 13

Attaching some screencasts to help on a current discussion on the code review.
cl_as_is.mp4
357 KB View Download
cl_xiyuan.mp4
297 KB View Download
cl_xiyuan_with_added_mirror.mp4
327 KB View Download

Comment 9 by manucornet@chromium.org, Dec 14

Cc: abodenha@chromium.org sdantul...@chromium.org manucornet@chromium.org mkarkada@chromium.org abod...@chromium.org omrilio@chromium.org kejiashao@chromium.org
 Issue 872985  has been merged into this issue.

Comment 10 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8c4efaa5e792c591f9c2f8e454b17b9b2d96262

commit f8c4efaa5e792c591f9c2f8e454b17b9b2d96262
Author: Manu Cornet <manucornet@chromium.org>
Date: Fri Dec 14 17:43:36 2018

CrOS Shelf: Fix RTL drag gesture

Before this CL, given the reverse X coordinates in RTL, a drag towards
one direction would cause the dragged view to move in the opposite
direction.

Bug:  913092 
Change-Id: I7454d9161d91d459cedcccd1e8da0a2b10918655
Reviewed-on: https://chromium-review.googlesource.com/c/1373452
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616730}
[modify] https://crrev.com/f8c4efaa5e792c591f9c2f8e454b17b9b2d96262/ash/shelf/shelf_view.cc

Comment 11 by manucornet@chromium.org, Dec 14

Labels: Merge-Request-72

Comment 12 by sheriffbot@chromium.org, Dec 15

Project Member
Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

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

Comment 13 by dgagnon@google.com, Dec 17

Labels: -Hotlist-Merge-Review -Merge-Review-72 Merge-Approved-72

Comment 14 by bugdroid1@chromium.org, Dec 17

Project Member
Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d2d2d3eceeeca2c43a85335bf3ff502dd3936d43

commit d2d2d3eceeeca2c43a85335bf3ff502dd3936d43
Author: Manu Cornet <manucornet@chromium.org>
Date: Mon Dec 17 20:17:08 2018

CrOS Shelf: Fix RTL drag gesture

Before this CL, given the reverse X coordinates in RTL, a drag towards
one direction would cause the dragged view to move in the opposite
direction.

Bug:  913092 
Change-Id: I7454d9161d91d459cedcccd1e8da0a2b10918655
Reviewed-on: https://chromium-review.googlesource.com/c/1373452
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616730}(cherry picked from commit f8c4efaa5e792c591f9c2f8e454b17b9b2d96262)
Reviewed-on: https://chromium-review.googlesource.com/c/1380542
Reviewed-by: Manu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#405}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/d2d2d3eceeeca2c43a85335bf3ff502dd3936d43/ash/shelf/shelf_view.cc

Comment 15 by manucornet@chromium.org, Dec 17

Status: Fixed (was: Started)

Comment 16 by cr-audit...@appspot.gserviceaccount.com, Dec 19

Project Member
Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision d2d2d3eceeeca2c43a85335bf3ff502dd3936d43 was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required --

Comment 17 by cr-audit...@appspot.gserviceaccount.com, Dec 19

Project Member
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/d2d2d3eceeeca2c43a85335bf3ff502dd3936d43

Commit: d2d2d3eceeeca2c43a85335bf3ff502dd3936d43
Author: manucornet@chromium.org
Commiter: manucornet@chromium.org
Date: 2018-12-17 20:17:08 +0000 UTC

CrOS Shelf: Fix RTL drag gesture

Before this CL, given the reverse X coordinates in RTL, a drag towards
one direction would cause the dragged view to move in the opposite
direction.

Bug:  913092 
Change-Id: I7454d9161d91d459cedcccd1e8da0a2b10918655
Reviewed-on: https://chromium-review.googlesource.com/c/1373452
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Manu Cornet <manucornet@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616730}(cherry picked from commit f8c4efaa5e792c591f9c2f8e454b17b9b2d96262)
Reviewed-on: https://chromium-review.googlesource.com/c/1380542
Reviewed-by: Manu Cornet <manucornet@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#405}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment