New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 604346 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : Focus is not fully seen at dropdown of Web & App Activity page

Project Member Reported by mm00333...@techmahindra.com, Apr 18 2016

Issue description

Version: 51.0.2704.15
OS: Chrome
Platform : 8172.4.0 (Official Build) dev-channel daisy,Peppy,Blaze

URL : https://history.google.com/history/

What steps will reproduce the problem?
(1)Sign into User -> Go to above URL
(2)Now click on Three Dot Menu beside calendar icon and hover the mouse on any option and observe the focus(For Ex: Observe Focus on Help Option)

Expected: Focus should be fully seen at dropdown of Web & App Activity page
Actual: Instead Focus is not fully seen at dropdown

This is Regression Issue as it is working fine in 49.0.2623.112/7834.70.0 stable-channel daisy.
 
Actual_Focus.png
75.5 KB View Download
Expected_Focus.png
78.0 KB View Download
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on Blaze using chrome version 51.0.2704.15/8172.4.0 

Comment 2 by tkent@chromium.org, Apr 18 2016

Components: -Blink>Forms>Select Blink
Labels: Needs-Bisect
It's not a SELECT form control.

Cc: cbiesin...@chromium.org
They're flex items (though I'm not sure that matters?)

They use width: 100% but get sized at less than 100%, for some reason. Looking forward to the bisect results...
Components: -Blink Blink>Focus
Hmm ... I don't actually see a red focus box at all, but I haven't tested this on CrOS, just on Mac and Linux. How are you getting the focus box to show up?

Setting this to Blink>Focus, though this might also be a layout issue ...
This is about the greyish background color, right? The red thing was just photoshopped in to show us what the issue is?
Cc: dpranke@chromium.org
+dpranke so he sees my reply above
@cbeisinger - I'm not sure, but you could be right. I can confirm that I saw the not-full-size backgrounds.
Components: -Blink>Focus Blink>Layout>Flexbox
So, yeah, maybe this is really a Flexbox issue ...

Comment 9 by e...@chromium.org, Apr 19 2016

Status: Available (was: Untriaged)
Cc: r...@igalia.com
Labels: -Needs-Bisect
Owner: cbiesin...@chromium.org
You are probably looking for a change made after 386044 (known good), but no later than 386048 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/7a247ee1edbb291fbbb5377a3b2073e1e83ce803..022699588a47674a7255f23daaa0574c8e91061e

Likely caused by rego's change:
https://chromium.googlesource.com/chromium/src/+/b75b9bdaa3ede6fe0230ae822885d6fb38ed514b

But that's weird. Let me investigate this a bit more...
Soo...

I think the new behavior is essentially correct, bug that  bug 341310 's pending codereview should have made this render as expected. I am not currently sure why it didn't.

Comment 12 by r...@igalia.com, Apr 20 2016

@cbiesinger I can confirm that without my patch the issue doesn't happen.

Actually you just need this diff to make it work again:
diff --git a/third_party/WebKit/Source/core/layout/LayoutBox.cpp b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
index 007c6f9..b914f8d 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -4183,7 +4183,7 @@ static bool logicalWidthIsResolvable(const LayoutBox& layoutBox)
         return true;
     if (box->isLayoutView())
         return true;
-    if (box->isOutOfFlowPositioned() && !box->style()->logicalLeft().isAuto() && !box->style()->logicalRight().isAuto())
+    if (box->isOutOfFlowPositioned())
         return true;
     if (box->hasOverrideContainingBlockLogicalWidth())
         return box->overrideContainingBlockContentLogicalWidth() != -1;

However, I'm not sure why the <md-menu-item> (which are flexboxes) are affected by that change,
as I don't see that they're absolutely positioned.
It's an ancestor of the md-menu-item that's positioned (positioned:fixed). The div class=md-open-menu-container.

Comment 14 by r...@igalia.com, Apr 28 2016

I've been taking a look, one easy fix in the CSS would be to add:
  md-menu-item > * {
    flex: 1;
  }

I've created a reduced test case to reproduce the issue:
http://jsbin.com/vadoha/1/edit?html,css,output

The problem is that the flex items have "width: 100%;"
which now seems to be resolved to "auto"
(similar to what we do for grid tracks I guess).
I'm not an expert on the flexbox spec so probably @cbiesinger should give his opinion.

BTW, it seems this can be a similar issue to this ongoing discussion in www-style:
https://lists.w3.org/Archives/Public/www-style/2016Apr/0329.html
I don't think that's the real issue -- what we have is this structure:

  OutOfFlow
    + Flexbox (column)
      + Flexbox (row)
        + 100% width

Now, the row flexbox is itself a flex item with align-self: stretch. That means its width should be considered definite.

This seems to be a bug in mainAxisLengthIsDefinite, which needs to take this case into account

Comment 16 Deleted

indonesia
30 Apr 2016 05.07, "cbiesinger@chromium.org via Monorail" <
monorail@chromium.org> menulis:

Comment 18 by r...@igalia.com, May 2 2016

@cbiesinger maybe it's not the same case, but what I'm seeing seems really similar.

Regular Block
|--> Flexbox (row)
   |--> Flex item 100% width

If the main block is positioned, then the width of the flexboxes seems to be considered indefinite, and the 100% in the items is resolved as auto.
I don't see the need of having the "Flexbox (column)" in the middle.
And using "flex: 1" seems to be fixing it for me.

New example:
http://jsbin.com/lifuki/1/edit?html,css,output

Anyway, as I said I don't know properly the flexbox spec and I don't know if the width should be considered definite or not in that situation.

Your case is working as expected. If the regular block is positioned then the flexbox will "inherit" the indefinite width and thus 100% can't be resolved. Same if you replace the flexbox with a regular div. That flex: 1 fixes this issue is also expected per the flexbox spec.

With the additional column flexbox, things are different because stretch alignment makes dimensions definite, again per the flexbox spec.
indonesia
Pada tanggal 2 Mei 2016 21.54, "cbiesinger@chromium.org via Monorail" <
monorail@chromium.org> menulis:

Comment 21 Deleted

OK, I have a simple fix: https://codereview.chromium.org/1943633002

Now all I need is a test...
Testcase!
history.html
533 bytes View Download
Project Member

Comment 24 by bugdroid1@chromium.org, May 5 2016

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

commit ec80ed8bf97986c752629bd0a550800fbe8bd82b
Author: rego <rego@igalia.com>
Date: Thu May 05 09:35:36 2016

Revert of [css-grid] Floated grid containers have indefinite width (patchset #1 id:1 of https://codereview.chromium.org/1892813002/ )

Reason for revert:
This is breaking how percentages work in
Flexbox and Grid compared to regular Blocks.

There're some discussion ongoing on the CSS WG to verify
the expected behavior, so we're reverting this for now
until we've a final resolution.

Original issue's description:
> [css-grid] Floated grid containers have indefinite width
>
> Fix LayoutBox::hasDefiniteLogicalWidth() to return FALSE for
> floated grid containers with auto width.
>
> This makes that percentage tracks are treated as "auto"
> for floated grid containers with indefinite width.
>
> BUG= 603854 
> TEST=fast/css-grid-layout/floated-grid-container-percentage-tracks.html
>
> Committed: https://crrev.com/a7f443f6a36a2a524c4c29a2ef1bb6d02249d7b2
> Cr-Commit-Position: refs/heads/master@{#388054}

TBR=cbiesinger@chromium.org,mstensho@opera.com,svillar@igalia.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 538513 , 603854 , 604346 , 608783 

Review-Url: https://codereview.chromium.org/1948203004
Cr-Commit-Position: refs/heads/master@{#391782}

[delete] https://crrev.com/5f1b82e8460a3d4d0a45346cdcdf5862be27c2c7/third_party/WebKit/LayoutTests/fast/css-grid-layout/floated-grid-container-percentage-tracks.html
[modify] https://crrev.com/ec80ed8bf97986c752629bd0a550800fbe8bd82b/third_party/WebKit/Source/core/layout/LayoutBox.cpp

Project Member

Comment 25 by bugdroid1@chromium.org, May 5 2016

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

commit 83e80002d146d1493ae16c1cb585cc458e955ee3
Author: rego <rego@igalia.com>
Date: Thu May 05 12:18:13 2016

Revert of [css-grid] Fix definite/indefinite size detection for abspos elements (patchset #2 id:20001 of https://codereview.chromium.org/1383003002/ )

Reason for revert:
This is breaking how percentages work in
Flexbox and Grid compared to regular Blocks.

There're some discussion ongoing on the CSS WG to verify
the expected behavior, so we're reverting this for now
until we've a final resolution.

Original issue's description:
> [css-grid] Fix definite/indefinite size detection for abspos elements
>
> We were considering that any abspos element has a definite size, and
> that's not true. That's only true if the offset properties in that
> dimension (left and right or top and bottom) are non-auto.
> Fixed LayoutBox::hasDefiniteLogicalWidth|Height() to check this
> properly.
>
> This has been confirmed by the CSS WG in the following thread:
> https://lists.w3.org/Archives/Public/www-style/2015Nov/0074.html
>
> BUG= 538513 
> TEST=fast/css-grid-layout/positioned-grid-container-percentage-tracks.html
>
> Committed: https://crrev.com/b75b9bdaa3ede6fe0230ae822885d6fb38ed514b
> Cr-Commit-Position: refs/heads/master@{#386045}

TBR=cbiesinger@chromium.org,svillar@igalia.com,mstensho@opera.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 538513 , 603854 , 604346 , 608783 

Review-Url: https://codereview.chromium.org/1954683002
Cr-Commit-Position: refs/heads/master@{#391789}

[delete] https://crrev.com/16d0abdd7230c89c7d0bf55e8de24a86fed39f40/third_party/WebKit/LayoutTests/fast/css-grid-layout/positioned-grid-container-percentage-tracks.html
[modify] https://crrev.com/83e80002d146d1493ae16c1cb585cc458e955ee3/third_party/WebKit/Source/core/layout/LayoutBox.cpp

Comment 26 by r...@igalia.com, May 5 2016

Hopefully this has been fixed after the reverts.
Project Member

Comment 27 by bugdroid1@chromium.org, May 6 2016

Labels: merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4cac97891d80a3f9789d4c5ae02a40ca4c9d3c21

commit 4cac97891d80a3f9789d4c5ae02a40ca4c9d3c21
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Fri May 06 17:24:14 2016

Revert of [css-grid] Fix definite/indefinite size detection for abspos elements (patchset #2 id:20001 of https://codereview.chromium.org/1383003002/ )

Reason for revert:
This is breaking how percentages work in
Flexbox and Grid compared to regular Blocks.

There're some discussion ongoing on the CSS WG to verify
the expected behavior, so we're reverting this for now
until we've a final resolution.

Original issue's description:
> [css-grid] Fix definite/indefinite size detection for abspos elements
>
> We were considering that any abspos element has a definite size, and
> that's not true. That's only true if the offset properties in that
> dimension (left and right or top and bottom) are non-auto.
> Fixed LayoutBox::hasDefiniteLogicalWidth|Height() to check this
> properly.
>
> This has been confirmed by the CSS WG in the following thread:
> https://lists.w3.org/Archives/Public/www-style/2015Nov/0074.html
>
> BUG= 538513 
> TEST=fast/css-grid-layout/positioned-grid-container-percentage-tracks.html
>
> Committed: https://crrev.com/b75b9bdaa3ede6fe0230ae822885d6fb38ed514b
> Cr-Commit-Position: refs/heads/master@{#386045}

TBR=cbiesinger@chromium.org,svillar@igalia.com,mstensho@opera.com
BUG= 538513 , 603854 , 604346 , 608783 

Review-Url: https://codereview.chromium.org/1954683002
Cr-Commit-Position: refs/heads/master@{#391789}
(cherry picked from commit 83e80002d146d1493ae16c1cb585cc458e955ee3)

Review URL: https://codereview.chromium.org/1957693002 .

Cr-Commit-Position: refs/branch-heads/2704@{#423}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[delete] https://crrev.com/179b1ad323b6bd741391eced5868c7876d65f536/third_party/WebKit/LayoutTests/fast/css-grid-layout/positioned-grid-container-percentage-tracks.html
[modify] https://crrev.com/4cac97891d80a3f9789d4c5ae02a40ca4c9d3c21/third_party/WebKit/Source/core/layout/LayoutBox.cpp

Status: Fixed (was: Available)
works in canary!
Project Member

Comment 29 by bugdroid1@chromium.org, May 10 2016

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

commit 9822f23e09df78a8a15380fbce9659397f83ee68
Author: cbiesinger <cbiesinger@chromium.org>
Date: Tue May 10 19:06:36 2016

[css-flexbox] Add test case for definite widths

Widths should always be definite, and this testcase
tests for that.

BUG= 604346 

Review-Url: https://codereview.chromium.org/1943633002
Cr-Commit-Position: refs/heads/master@{#392669}

[add] https://crrev.com/9822f23e09df78a8a15380fbce9659397f83ee68/third_party/WebKit/LayoutTests/css3/flexbox/bug604346-expected.html
[add] https://crrev.com/9822f23e09df78a8a15380fbce9659397f83ee68/third_party/WebKit/LayoutTests/css3/flexbox/bug604346.html

Labels: VerifyIn-54

Comment 31 by ka...@chromium.org, Aug 31 2016

Labels: Bulk-Verified
Status: Verified (was: Fixed)

Sign in to add a comment