New issue
Advanced search Search tips

Issue 840150 link

Starred by 16 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Tabs disappear behind other tabs

Reported by david.st...@gmail.com, May 6 2018

Issue description

UserAgent: Mozilla/5.0 (X11; CrOS aarch64 10649.0.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3421.0 Safari/537.36
Platform: 10649.0.0 (Official Build) canary-channel elm

Steps to reproduce the problem:
1. Happens when I close a tab.
2. 
3. 

What is the expected behavior?
Don't hide the tabs

What went wrong?
I usually have 20 to 30 tabs open at any one time. The first 15 or so are pinned. It happens most frequently when I close one of the non-pinned tabs. The remaining non-pinned tabs slide to the left covering the pinned tabs. When I move the leftmost tab to the right the pinned tabs slide back into view.I sent a feedback report a few minutes ago.

Did this work before? Yes Perhaps the previous update

Chrome version: 68.0.3421.0  Channel: n/a
OS Version: 10649.0.0
Flash Version: 29.0.0.171
 
Screenshot 2018-05-05 at 7.10.43 PM.png
149 KB View Download
Screenshot 2018-05-05 at 7.10.59 PM.png
170 KB View Download
In the latest screenshot, you can see pinned tabs seemingly out of place. I found that if I clicked between the unpinned tabs, I could select a pinned tab. 
Screenshot 2018-05-08 at 10.56.15 AM.png
47.6 KB View Download
Cc: afakhry@chromium.org omrilio@chromium.org pkasting@chromium.org
Components: -UI UI>Browser>TabStrip
Labels: -Pri-2 Pri-1
Owner: malaykeshav@chromium.org
Status: Assigned (was: Unconfirmed)
Cc: malaykeshav@chromium.org
Labels: ReleaseBlock-Stable M-67
Owner: erikc...@chromium.org
This seems to be caused due to this change:
https://chromium.googlesource.com/chromium/src/+/dc48c25ef9188147c14cae55c690be18ec2a29f6

Assigning to author.
 malaykeshav: Could you clarify how you know that the CL is responsible? Do you have a bisect? Is this a gut instinct?
david: Is this on a touch device?
I performed a manual bisect. It was working on the CL before that change. 

But I might have messed up with the bisect. Let me double check.
No need to double check, I was just wondering if you have a repo? [does this require a CrOS touch device?]
repro steps:

"""
To repro the bug you need to have CrOS device with touchable chrome enabled. 
--top-chrome-md=material-touch-optimized

And then pin a couple of tabs. Once the tab stacking starts, use the tab close button to close one of the tabs. 
"""
Cc: erikc...@chromium.org
Owner: pkasting@chromium.org
malaykeshav performed another bisect and got this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1024666

I'm pretty sure the problem lines are: 
https://chromium-review.googlesource.com/c/chromium/src/+/1024666/4/chrome/browser/ui/views/tabs/tab_strip_layout.cc#b68

CalculateBoundsForPinnedTabs used to calculate the bounds for the first non-pinned tab. The new CL skips that calculation. However, GenerateIdealBoundsForPinnedTabs [which calls CalculateBoundsForPinnedTabs] still uses the bounds for the first non-pinned tab:

https://cs.chromium.org/chromium/src/chrome/browser/ui/views/tabs/tab_strip.cc?type=cs&q=tab_strip.cc&sq=package:chromium&g=0&l=2220

"""  
CalculateBoundsForPinnedTabs(GetTabSizeInfo(), num_pinned_tabs, tab_count(),
                               start_x, &tab_bounds);
...
  return (num_pinned_tabs < tab_count())
             ? tab_bounds[num_pinned_tabs].x()
             : tab_bounds[num_pinned_tabs - 1].right() - Tab::GetOverlap();
Yeah, I saw today that that was broken; didn't realize I was the one who broke it :).  I was actually already fixing that issue.  However, my fix is going to be impossible to merge back, so I'll need to determine a separate, standalone fix for this.
Oh wait, this is dev/canary-only.  OK, no worries about merging then.
Not used to seeing so much activity on one of my reports. There were so many Chrome crashes in the latest Canary, I temporarily switched to Dev but I think I saw this earlier today in Dev. I will update.
Another piece to this is at some point my non-pinned tabs stack at one end. Instead of becoming smaller as I add a new tab, they become full size and the "excess" tabs become stacked. The stacking can occur at either end of the tabs.
Screenshot 2018-05-16 at 9.19.22 AM.png
26.7 KB View Download
#13
This is expected with the new UI on chrome OS. 
So it isn't easy without touch. This should go to a feature request but what would make the stacking useful is a dropdown list of open tabs. Even the larger tab, not enough of the site name is exposed to determine a specific tab.
Yes. The interaction with mouse isnt easy for stacked tabs. There are plans to change this in the next release.
Labels: -ReleaseBlock-Stable -M-67 M-68
This is a usability issue and we are releasing touchable chrome on 4 devices for M67. Shouldn't this be a release blocker for M67?
The CL in comment 3 isn't in the M67 branch, is it?  It landed after the branch point and hasn't been merged.
Ah, Right! I didn't see that.

Comment 21 Deleted

#16
Is there a bug for the plans already? Would you post the issue number here when it is available? Thank you in advance. Been extremely concerned about the future of stacked tabs after MD refresh.
Can we take the discussion of stacked tabs elsewhere?  It's not related to this bug.
Of course, that 'elsewhere' is what I asked for... Just a number, no discussion.
I know the rules well and have no intention to disrupt this bug, but this is the only place I can directly contact a known member who knows about the plans. I am sorry if this annoys already.
Sorry, I spoke too harshly.  I don't know what, if any, plans there are for stacked tabs and how MD refresh would affect them.  If something is broken that you want fixed, the best route is probably to file a bug.  If not, and you're simply concerned about the future, I would say to wait until refresh is done enough to be on by default in some channel for a touchable device, and then share feedback/bugs on the stacked experience is at that point.
Is there somewhere I can comment on the stacked tabs?
As noted, if you think something is wrong today, I would simply file a new bug.
 Issue 840020  has been merged into this issue.

Comment 29 by dymp...@gmail.com, May 19 2018

They're stacking in Beta 67 as well.

844824 - Can only see 8 tabs without overlap on Pixelbook, no way to scroll tab strip with trackpad - chromium - Monorail
https://bugs.chromium.org/p/chromium/issues/detail?id=844824

840150 - Tabs disappear behind other tabs - chromium - Monorail
https://bugs.chromium.org/p/chromium/issues/detail?id=840150

844495 - Tabstrip in stacked mode even during mouse interaction - chromium - Monorail
https://bugs.chromium.org/p/chromium/issues/detail?id=844495

Comment 30 by dymp...@gmail.com, May 19 2018

Sorry, forgot to add my chrome://version


Google Chrome	67.0.3396.49 (Official Build) beta (32-bit)
Revision	5b1d0ea887316c0d486d1df5572e04d7813976a1-refs/branch-heads/3396@{#612}
Platform	10575.40.0 (Official Build) beta-channel veyron_minnie
Firmware Version	Google_Veyron_Minnie.6588.237.0
ARC	4782370
JavaScript	V8 6.7.288.33
Flash	29.0.0.171 /opt/google/chrome/pepper/libpepflashplayer.so
User Agent	Mozilla/5.0 (X11; CrOS armv7l 10575.40.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.49 Safari/537.36


@29: One of those is this bug, and the other two are dupes of an unrelated bug.  Stacked mode being on in M67 when it shouldn't be is not the same as this.
 Issue 845238  has been merged into this issue.
I have just moved back to Canary from Dev a few minutes ago. At least once today I saw tab slide totally to the left covering my pinned tabs. I tried to take a screenshot but the tabs moved back to the right as I attempted the process.
Yep, this bug is known not to be fixed yet.  The cause is well-understood, I just haven't reached the point of landing a mergeable fix, because I'm been stacked up with other stuff.
Project Member

Comment 35 by bugdroid1@chromium.org, Jun 4 2018

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

commit 4ff23e18e3a345031336435002a5cbefee1cf45a
Author: Peter Kasting <pkasting@chromium.org>
Date: Mon Jun 04 18:55:13 2018

Fix uninitialized read during pinned tab layout.

When I changed the API to return the pinned tab X instead of storing it in the
next array element, I failed to update all the callers.

BUG= 840150 

Change-Id: Ib4ec2736bbdf1b768d318c2d2f7e214d3ae3929f
Reviewed-on: https://chromium-review.googlesource.com/1083778
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564170}
[modify] https://crrev.com/4ff23e18e3a345031336435002a5cbefee1cf45a/chrome/browser/ui/views/tabs/tab_strip.cc

Labels: Merge-Request-68
Project Member

Comment 37 by sheriffbot@chromium.org, Jun 5 2018

Labels: -Merge-Request-68 Hotlist-Merge-Approved Merge-Approved-68
Your change meets the bar and is auto-approved for M68. Please go ahead and merge the CL to branch 3440 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Fixed (was: Assigned)
Landed in https://chromium-review.googlesource.com/c/chromium/src/+/1094274 .
Project Member

Comment 39 by bugdroid1@chromium.org, Jun 9 2018

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed68a26ea15c66434ca671d591a7f57522951a77

commit ed68a26ea15c66434ca671d591a7f57522951a77
Author: Peter Kasting <pkasting@chromium.org>
Date: Sat Jun 09 02:08:12 2018

Fix uninitialized read during pinned tab layout.

When I changed the API to return the pinned tab X instead of storing it in the
next array element, I failed to update all the callers.

BUG= 840150 

Change-Id: Ib4ec2736bbdf1b768d318c2d2f7e214d3ae3929f
Reviewed-on: https://chromium-review.googlesource.com/1083778
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#564170}(cherry picked from commit 4ff23e18e3a345031336435002a5cbefee1cf45a)
Reviewed-on: https://chromium-review.googlesource.com/1094274
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#266}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/ed68a26ea15c66434ca671d591a7f57522951a77/chrome/browser/ui/views/tabs/tab_strip.cc

Sign in to add a comment