New issue
Advanced search Search tips

Issue 876642 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

TabGrid menu on the TabSwitcher isn't displayed correctly

Project Member Reported by gambard@chromium.org, Aug 22

Issue description

iPad only.

What steps will reproduce the problem?
(1) Long Press on the TabGrid icon in the TabStrip
(2) Release the press

What is the expected result?
The Popup Menu should be displayed.

What happens instead?
A new tab is opened. This is because the Popup Menu is displayed on top of the TabGrid button, so releasing the press actually triggers the first action item.

The issue is that the popup menu is wrongly positioned. It should be positioned lower. This is because the layout guide is positioned wrongly.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 22

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

commit 7fc059fcdda0de0f724fef8ce444835d295c3dad
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Aug 22 09:44:21 2018

[iOS] Fix positioning of the TabStrip LayoutGuide

The positioning of the LayoutGuide named kTabStripTabSwitcherGuide in
the TabStrip was wrong because the frame wasn't converted to the layout
guide owning view.
It leads to an incorrect positioning, as the width of the button and the
height of the status bar weren't taken into account.

Bug:  876642 ,  875760 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I537976c049508e3f0ac4a2e52ef654333a9ce1b3
Reviewed-on: https://chromium-review.googlesource.com/1184706
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584981}
[modify] https://crrev.com/7fc059fcdda0de0f724fef8ce444835d295c3dad/ios/chrome/browser/ui/tabs/tab_strip_controller.mm

Cc: srikanthg@chromium.org kariahda@chromium.org
Labels: Merge-Request-69
Status: Fixed (was: Assigned)
There is still a small vertical offset if the menu is displayed right after a cold start (I have created  issue 876982  to track it).
Otherwise, looks good.

Same merge request as  issue 875760 .
Project Member

Comment 3 by sheriffbot@chromium.org, Aug 23

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: Less than 8 days to go before AppStore submit on M69
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

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

Comment 4 by bugdroid1@chromium.org, Aug 23

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

commit 7be2d605e768d35fd5051d6ac51bce7c0115005a
Author: Gauthier Ambard <gambard@chromium.org>
Date: Thu Aug 23 21:45:23 2018

[iOS] Fix positioning of the TabStrip LayoutGuide

The positioning of the LayoutGuide named kTabStripTabSwitcherGuide in
the TabStrip was wrong because the frame wasn't converted to the layout
guide owning view.
It leads to an incorrect positioning, as the width of the button and the
height of the status bar weren't taken into account.

Bug:  876642 ,  875760 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I537976c049508e3f0ac4a2e52ef654333a9ce1b3
Reviewed-on: https://chromium-review.googlesource.com/1184706
Reviewed-by: Mark Cogan <marq@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#584981}(cherry picked from commit 7fc059fcdda0de0f724fef8ce444835d295c3dad)
Reviewed-on: https://chromium-review.googlesource.com/1187166
Reviewed-by: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#793}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/7be2d605e768d35fd5051d6ac51bce7c0115005a/ios/chrome/browser/ui/tabs/tab_strip_controller.mm

Labels: -Merge-Review-69 Merge-Approved-69
Labels: -Hotlist-Merge-Review
Labels: -Merge-Approved-69
Status: Verified (was: Fixed)
Verified the issue on the build 69.0.3497.70beta tested on iPad Mini iOS 11 and iPad Pro iOS 11.4.1

The popup layout is slightly positioned lover to the tabswitcher icon, Looks good 

Images :
https://drive.google.com/corp/drive/u/0/folders/1ObHbprD9jiZe2KbNRB4yRCpSNOoCo2E2


Sign in to add a comment