Selecting any menu option on Google Plus mobile site on FxAndroid does nothing

VERIFIED FIXED

Status

Tech Evangelism
Mobile
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: jsmith, Assigned: karlcow)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [country-all][css][sitewait], URL)

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
STR

1. Go to Google Plus's mobile site on FxAndroid
2. Select the menu option
3. Try to select any menu option available

Expected

Selecting a menu option should take you to the relevant area.

Actual

Nothing happens. The menu options appear to do nothing when clicked.
Hmm, I can see the layout is a little borked (g+ icon seems to be busting out of its container, menu icon too far left), but apart from that the menu WFM. Will investigate the layout stuff.

Jason, do you see the same thing?
(Reporter)

Comment 5

5 years ago
(In reply to Mike Taylor [:miketaylr] from comment #4)
> Hmm, I can see the layout is a little borked (g+ icon seems to be busting
> out of its container, menu icon too far left), but apart from that the menu
> WFM. Will investigate the layout stuff.
> 
> Jason, do you see the same thing?

Yup, those screenshots reflect what I saw.
Apparently reading comprehension is not my thing. >_< Confirmed that the menu items don't do anything when clicking them.
For the menu items, it turns out you just have to be very careful about where you tap for them to work. There's an :active style on the containing li that transitions in the gray background color, but if you're not clicking basically on the icon nothing will happen.

Here's the markup for a sample menu item:

<li class="xQ">
  <a href="/app/basic/people?cbp=14cupel1e7chg&amp;partnerid=gpmobile:android&amp;sview=11" class="M8a" id="147">
    <div class="DCb">
      <div role="presentation" class="Gu ARa u1a"></div>
    </div>
    <div class="L8a">People</div>
</a></li>

The <a> doesn't have any explicit width set, so it defaults to auto. According to the devtools Box Model panel, that's 72px.

The two childen divs (the icon and text) are display: table-cell.

Will try to reduce.
Created attachment 818773 [details]
googleplus_menu.html

Minimal test case. The width of the <a> stretches to the width of the containing <li> in Chrome, in Firefox it's constrained to the width of its children content. Need to read some specs before knowing which is correct.
David, do you have any insight into the difference between Chrome and Firefox here?
Do Chrome and Firefox produce the same parse tree for the malformed HTML?
Flags: needinfo?(dbaron)
Yeah, same parse tree. And I believe the malformed HTML is my own mistake, will upload a version with a correct <a> wrapping (but it makes no difference in the results).
Adding display:block to the wrapping <a> solves the problem. But it seems like the inconsistent behavior between Gecko and Chrome here isn't desirable.
Whiteboard: [country-all][contactready][css]
Adding an explicit width of 34px to the a.Bra that wraps the hamburger menu and the G+ icon fixes the wrapping issue (another instance of a div with display:table-cell being wrapped by an anchor).
hmm. What other browsers are doing?
I don't have access to Windows to test IE, but:

Width of <a> stretches to child content:
Firefox
Opera (Presto)

Width of <a> stretches to containing <li>:
Opera (Blink)
Chrome
Safari
Also an issue for FxOS, as reported in 946441.
Created attachment 8346086 [details]
table-version.html

Assuming that the div with display:table-cell creates anonymous TR and TABLE objects that behave like real TR and TABLE elements (no idea if that assumption is correct), I would expect a version with actual table elements to behave the same.

However, as one can observe in table-version.html, the A element extends to the width of the parent LI which matches the behavior of Chrome and Presto Opera. So it feels like our handling of the G+ page here is a core bug.
Summary: Selecting any menu option on Google Plus mobile site on FxAndroid does nothing → a element with display: table-cell children does not expand to the width of parent li element
Component: Mobile → Layout: Tables
Product: Tech Evangelism → Core
Whiteboard: [country-all][contactready][css] → [country-all][css]
(In reply to Mike Taylor [:miketaylr] from comment #19)
> Created attachment 8346086 [details]
> table-version.html
> 
> Assuming that the div with display:table-cell creates anonymous TR and TABLE
> objects that behave like real TR and TABLE elements (no idea if that
> assumption is correct), I would expect a version with actual table elements
> to behave the same.

The assumption isn't quite correct. The difference is that <table> is block-level, whereas display:table-cell will get wrapped in a block-level or inline-level table depending on the parent type (and in this case it gets wrapped in an inline-level table).

More details:
By default, <table> is block-level, so when placed inside of an <a>, it triggers a block-in-inline split, which makes the <a> generate a block-level box for the table to live inside of. And that block-level box is the thing that is clickable (due to the fact that it's part of the <a>) and as wide as the container (due to it being block-level and having the default block sizing behavior).

In contrast -- "display: table-cell" gets wrapped in either a block-level table or an inline-level table, depending on the type of its parent. That determination happens here:
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp?rev=73d6f276f58d&mark=9120-9122#9115
In this case, the parent (<a>) is inline-level, so we wrap it in an *inline-table*. So we don't end up with any full-container-width clickable block.

(I'm not sure if that wrapper-type-depending-on-the-parent behavior is correct per spec or not; I'm just describing what happens in Gecko, from a bit of prodding.)
(In reply to Daniel Holbert [:dholbert] from comment #20)
> (I'm not sure if that wrapper-type-depending-on-the-parent behavior is
> correct per spec or not; I'm just describing what happens in Gecko, from a
> bit of prodding.)

Looks like this is correct -- the spec explicitly calls for this.
 # 3. Generate missing parents:
 # [...]
 #   2. For each proper table child C in a sequence of consecutive
 #   proper table children, if C is misparented then generate
 #   an anonymous 'table' or 'inline-table' box T around C [...].
 #   (If C's parent is an 'inline' box, then T must be an
 #   'inline-table' box; otherwise it must be a 'table' box.) 
http://www.w3.org/TR/CSS21/tables.html#anonymous-boxes
Created attachment 8346204 [details]
testcase that demonstrates Chrome having a bug

Here's a testcase that demonstrates that Chrome has a bug here. This just tests the spec text from comment 21 -- comparing a table-cell inside of an inline-level <span> (which should generate an anonymous inline-table parent), vs. a table-cell with an explicit inline-table parent.

They should look the same, but in Chrome, they don't -- it does a block-in-inline split in the first case, which indicates that it's generating a block-level table.
I filed http://code.google.com/p/chromium/issues/detail?id=327832 to report this to the Blink folks.

In the meantime, this is back to a Tech Evang bug, since we're following the spec and it looks like the site is just coded to rely on the (incorrect) Blink behavior.
Assignee: nobody → english-us
Component: Layout: Tables → English US
Product: Core → Tech Evangelism
Version: unspecified → Trunk
Thanks Daniel. :) I learned quite a bit from your comments.

I also finally got a windows VM to test and IE10 matches our behavior. So it looks like Google is relying on WebKit/Chrome's behavior here and other browsers get the less-than-good experience.
Again, to be more clear, from Comment 13: we should ask Google to add "display: block" to the containing <a>, which (I just learned) means we'll wrap the display:table-cell content with a block-level table.
Whiteboard: [country-all][css] → [country-all][css][contactready]
Assignee: english-us → nobody
Component: English US → Mobile
(In reply to Mike Taylor [:miketaylr] from comment #25)
> We should ask Google to add
> "display: block" to the containing <a>

Agreed.

> which (I just learned) means we'll
> wrap the display:table-cell content with a block-level table.

True -- but more importantly, it means that the block for the <a> element *itself* will be the full width of its container. (even though the table will be skinnier)  That full width -- the width of the newly-block-level <a> -- would then be clickable.
Daniel, do we need a bug report on WebKit here too?  I expect we do...
Contacted Alex (Google)
Assignee: nobody → kdubost
Whiteboard: [country-all][css][contactready] → [country-all][css][sitewait]
(In reply to Boris Zbarsky [:bz] from comment #27)
> Daniel, do we need a bug report on WebKit here too?  I expect we do...

Probably, yeah. I filed https://bugs.webkit.org/show_bug.cgi?id=125640
Summary: a element with display: table-cell children does not expand to the width of parent li element → Selecting any menu option on Google Plus mobile site on FxAndroid does nothing
From Google

> The team submitted a fix for the bug today, so it should appear within the next week.
Status: NEW → ASSIGNED
This is fixed.
Thanks to Google.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Great to hear, thanks Karl (and Google).
You need to log in before you can comment on or make changes to this bug.