DescriptionFix a variety of tabstrip positioning issues.
These were primarily due to some extremely confused code in the glass frame's GetBoundsForTabStrip() function. After puzzling over this for a few hours I ended up rewriting it entirely, to hopefully be clearer. As part of this, the glass frame now unconditionally calls LayoutIncognitoIcon() to set the |incognito_bounds_|, in particular the X coordinate, which GetBoundsForTabStrip() can then rely on more consistently. This meant having to account for things like the new avatar button in LayoutIncognitoIcon(), which is somewhat weird since the incognito icon and the new avatar button never appear simultaneously, but it makes the code in GetBoundsForTabStrip() nicer.
This also adjusts the Ash frame, though only for the first two bullets below. (The others don't apply since Ash supports neither RTL nor the new avatar button.) It doesn't adjust the opaque frame; I'll check and adjust that as necessary later. The Ash and glass changes for the first two bullets here have to be done in sync since both use the same layout constants; the opaque frame does not yet use them.
Functional changes:
* The tabstrip was positioned wrongly against the incognito icon or (when no icon was visible) window edge. The Ash frame placed the tabstrip at 0 px from the window edge or 2 px from the incognito icon; the glass frame placed the tabstrip at -2 px from both. In all cases, the correct value should have been -6 px. At least for the no-incognito glass case, I think this broke during the addition of the new avatar button; I didn't check closely. The effect of this change will be to push the tabstrip much more flush against the window or icon edge.
* The material-mode tabstrip was also positioned wrongly against both of these (2 px instead of the desired 4 px), so fixed that while I was touching this. Since the MD tabstrip isn't in the tree yet it's basically impossible to visually verify that one or the other of these is more correct; trust me :). I also fixed the MD incognito icon vertical position for the glass frame because why not.
* In RTL mode, the tabstrip was positioned at 0 px from the caption buttons instead of -6. Set it to -6 to match LTR. However, for maximized RTL windows in non-MD, left the value at 0 instead of -6 to avoid having TabStrip's alpha-blending code make the new avatar button look glitchy. This will be better solved in the MD world by restricting the alpha blend more closely to the tab shapes and not letting it extend through the bounds of the tabstrip, but this hack works for now.
* In non-RTL mode with a very narrow new avatar button visible, added code to prevent the new tab button from sliding under the avatar button so far it would wind up closer to the caption buttons than if the avatar button hadn't been there.
* In RTL mode, the end (i.e. left side; the new tab button) of the tabstrip was spaced against the window edge minus the LTR caption button padding offsets, which didn't make sense since there are no caption buttons there. Instead, in both modes, let the tabstrip go up to the edge of the nonclient border, which seems like the closest match for how we treat the left window edge in LTR mode.
BUG=none
TEST=Windows 7/10 and CrOS windows should now have the tabstrip near-flush against the left edge of the toolbar in both LTR and RTL modes.
R=sky@chromium.org
Committed: https://chromium.googlesource.com/chromium/src/+/10a1865416e06c89831afd213b1e2212172d4da6
Patch Set 1 #Patch Set 2 : Remove erroneous comment #
Depends on Patchset: Messages
Total messages: 5 (1 generated)
|