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

Status: VERIFIED FIXED

(Web Compatibility :: Site Reports, defect)

Description

[Jason Smith [:jsmith]]

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.

Comment 1

[Mike Taylor [:miketaylr]]

Attachment: g+dropdown.png

Comment 2

[Mike Taylor [:miketaylr]]

Attachment: g+home.png

Comment 3

[Mike Taylor [:miketaylr]]

Attachment: g+menu.png

Comment 4

[Mike Taylor [:miketaylr]]

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?

Comment 5

[Jason Smith [:jsmith]]

(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.

Comment 6

[Mike Taylor [:miketaylr]]

Apparently reading comprehension is not my thing. >_< Confirmed that the menu items don't do anything when clicking them.

Comment 7

[Mike Taylor [:miketaylr]]

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.

Comment 8

[Mike Taylor [:miketaylr]]

Attachment: 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.

Comment 9

[Mike Taylor [:miketaylr]]

David, do you have any insight into the difference between Chrome and Firefox here?

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Do Chrome and Firefox produce the same parse tree for the malformed HTML?

Comment 11

[Mike Taylor [:miketaylr]]

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).

Comment 12

[Mike Taylor [:miketaylr]]

Attachment: g+menu_fixed.html

Comment 13

[Mike Taylor [:miketaylr]]

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.

Comment 14

[Mike Taylor [:miketaylr]]

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).

Comment 15

[Karl Dubost💡 :karlcow]

hmm. What other browsers are doing?

Comment 16

[Mike Taylor [:miketaylr]]

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

Comment 18

[Mike Taylor [:miketaylr]]

Also an issue for FxOS, as reported in 946441.

Comment 19

[Mike Taylor [:miketaylr]]

Attachment: 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.

Comment 20

[Daniel Holbert [:dholbert]]

(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.)

Comment 21

[Daniel Holbert [:dholbert]]

(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

Comment 22

[Daniel Holbert [:dholbert]]

Attachment: 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.

Comment 23

[Daniel Holbert [:dholbert]]

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.

Comment 24

[Mike Taylor [:miketaylr]]

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.

Comment 25

[Mike Taylor [:miketaylr]]

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.

Comment 26

[Daniel Holbert [:dholbert]]

(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.

Comment 27

[Boris Zbarsky [:bzbarsky]]

Daniel, do we need a bug report on WebKit here too?  I expect we do...

Comment 28

[Karl Dubost💡 :karlcow]

Contacted Alex (Google)

Comment 29

[Daniel Holbert [:dholbert]]

(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

Comment 30

[Karl Dubost💡 :karlcow]

From Google

> The team submitted a fix for the bug today, so it should appear within the next week.

Comment 32

[Karl Dubost💡 :karlcow]

This is fixed.
Thanks to Google.

Comment 33

[Mike Taylor [:miketaylr]]

Great to hear, thanks Karl (and Google).