New issue
Advanced search Search tips

Issue 875760 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

LongPress tip isn't displayed correctly on iPad

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

Issue description

iPad only.
Enable InProductHelp to IPH_LongPressToolbarTip.

What steps will reproduce the problem?
(1) Relaunch the app

What is the expected result?
A bubble should be presented, pointing to the TabGrid icon in the TabSwitcher.

What happens instead?
The bubble is presented in the top left corner.
 
Cc: kariahda@chromium.org
Labels: ReleaseBlock-Stable M-69
We want to display the tips on M69, so I think this is RBS.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 20

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

commit 58dbdef1aa71ca30d74266bbbaa6b6c5d529343c
Author: Gauthier Ambard <gambard@chromium.org>
Date: Mon Aug 20 12:59:28 2018

[iOS] Adjust LongPress tips on iPad

On iPad, the LongPress tip should use the the kTabStripTabSwitcherGuide
instead of the kTabSwitcherGuide.

Bug:  875760 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I3dce36249a9b766e79473506770dd8fe565a0c28
Reviewed-on: https://chromium-review.googlesource.com/1180968
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584416}
[modify] https://crrev.com/58dbdef1aa71ca30d74266bbbaa6b6c5d529343c/ios/chrome/browser/ui/bubble/bubble_presenter.mm

Labels: Merge-Request-69
Status: Verified (was: Assigned)
Verified on iPad 6 iOS 11.3.
Asking for merge.
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 21

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: Less than 10 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
For english the bubble is displayed correctly but for HE language the bubble still placed incorrectly. gambard@ can you please let me know if that needs tobe filed a new bug?

Please check the screenshots.
https://drive.google.com/corp/drive/u/0/folders/1cBiCO-3Cq_gncblEKinIPmXrr3CsXQzS

Tested on iOS11.4.1 iPad Pro, M70.0.3529.0 canary
Status: Assigned (was: Verified)
The issue is with the layout guide. I have filled  issue 876642  to track this, I will re-open this one too.
Project Member

Comment 7 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
Status: Fixed (was: Assigned)
I have verified on iPad 6, iOS 11.3, Hebrew and English.
There is the same small offset as  issue 876642  when you enter/leave tabgrid (with the done button) after a cold start and without interacting with the app.
I think it is fine to merge as is.
+srikanthg@ for verifying.
Status: Verified (was: Fixed)
Longpress tip displayed correctly on iPads for EN and HE.
Attached screenshot for HE.
https://drive.google.com/file/d/1EONnfiRN04Ak3A6NRrJpw-DTE9v44yJY/view

Verified on M70.0.3531.0 canary iPad Pro, iOS 11.4.1
Please approve for merge.
Approved. I will merge so this makes tonight's build.
Project Member

Comment 11 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

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 23

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

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

[iOS] Adjust LongPress tips on iPad

On iPad, the LongPress tip should use the the kTabStripTabSwitcherGuide
instead of the kTabSwitcherGuide.

Bug:  875760 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I3dce36249a9b766e79473506770dd8fe565a0c28
Reviewed-on: https://chromium-review.googlesource.com/1180968
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#584416}(cherry picked from commit 58dbdef1aa71ca30d74266bbbaa6b6c5d529343c)
Reviewed-on: https://chromium-review.googlesource.com/1187343
Reviewed-by: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#794}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/d6bae343f6e900bfb20c9ff52703c03071f0341a/ios/chrome/browser/ui/bubble/bubble_presenter.mm

Labels: -Merge-Review-69 Merge-Approved-69
Labels: -Hotlist-Merge-Review
Labels: -Merge-Approved-69
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 tab switcher icon, Looks good 
Tested in English and Hebrew language

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


Sign in to add a comment