1302842 - Facebook edit suggestion pages have overlapped layout

Status: RESOLVED DUPLICATE of bug 1196668

(Core :: Layout, defect, P3)

Description

[Tobias K.]

Attachment: Screenshot_2016-09-14-21-22-44.png

I am using the web interface of Facebook. Usually I report when shops/restaurants are closed. There is a special page within Facebook to report this. Sequence: Look for a shop/restaurant. For example "Wein & Cafe Kult" in Unterhaching. Then, select "more". Select "Änderung vorschlagen"/"propose change". Select "Öffnungszeiten"/"Opening times". Then, you will see attached screen, which is almost unreadable. Is it possible to improve?

Comment 1

[Alex Caziuc]

Thanks for the info Tobias.

I was able to reproduce this using Sony Xperia Z5(6.0.1) with 48 Release.

Steps:
1. Go to https://m.facebook.com
2. Search for a restaurant - 'Wein & Cafe Kult'
3. Tap 3 dotted More -> Suggest Edits -> tap edit on Opening Hours -> Schedule

Actual result:
The text is unreadable as in attachment

Expected result:
The text should be readable

Note: It works on Chrome

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

This is likely a tech evangelism issue.

Comment 3

[Mike Taylor [:miketaylr]]

Hi Tobias,

Would you be able to test if the issue still reproduces in Beta 49 (or, wait a day until 49 is released then re-test on stable)?

Comment 4

[Tobias K.]

Hi! Thanks for taking care. I'll retest as soon as 49 is available and let you know the result. Cheers

Comment 5

[Tobias K.]

So, today I retested based on v49, which just came in. Yet, issue still existing. Same behavior as v48. Just friendly remark: As it is easy to reproduce, why do you ask the problem finder to reproduce? Anyway, as I mentioned, it's resproducible also with v49.
Cheers

Comment 6

[Mike Taylor [:miketaylr]]

Thanks for re-testing Tobias.

> Just friendly remark: As it is easy to reproduce, why do you ask the problem finder to reproduce? 

Because I wasn't able to reproduce locally.

Comment 7

[Mike Taylor [:miketaylr]]

Ah, OK. Now I see it. I had missed the "Schedule" part of Step 3 from Comment #1.

Comment 8

[Mike Taylor [:miketaylr]]

Attachment: Layout soup in Desktop

This reproduces in desktop as well, as long as you spoof the UA string.

Here's the markup for a single row:

<div class="_5cnm _1dd9">
  <div data-sigil="touchable" class="_3qok _5aj7">
    <div class="_4bl9">
      <div class="_3qok _5rgx">
        <div class="_1yjv">
          <div class="_1yk0 _1yk1">
            <div>
              <div class="_49r2  _52jd _52jb _52jg">
                <img>
                <div class="_49r3">Arts &amp; Entertainment</div>
              </div>
              <div class="_5cnj">
                <div class="_49r2  _52jc _52j9 _52jg">
                  <img>
                  <div class="_49r3">Movie Theater, Museum, Sports Venue &amp; Stadium</div>
                </div>
              </div>
            </div>
          </div>
        </div>
      </div>
    </div>
    <div class="_4bl7">
      <div class="_3qok _3qoq">
        <div class="_1yjv">
          <div class="_1yk0 _1yk1 _1yk2"><i alt="" class="img sp_oQYUStcX53B sx_df4086"></i>
          </div>
        </div>
      </div>
    </div>
  </div>
</div>

Comment 9

[Mike Taylor [:miketaylr]]

What's funky is "._1yk0._1yk1 > div" is 3px high in Firefox and 39px in Chrome. Those bare <img>s are also 0 x 14.5px in Firefox and 0 x 0 in Chrome. Not sure what's up with that yet.

Comment 10

[Mike Taylor [:miketaylr]]

Hm, using a tool to extract the computed view from Chrome.... it looks the same in Firefox: http://jsfiddle.net/b72vkhns/

Might mean there's some JS at play.

Comment 11

[Mike Taylor [:miketaylr]]

If I disable `position: absolute` from the following rule, the page displays the same as Chrome.

._49r3 {
    left: 0;
    overflow: hidden;
    position: absolute;
    text-overflow: ellipsis;
    top: 0;
    white-space: nowrap;
    width: 100%;
}

Now to figure out why...

Comment 12

[Tobias K.]

Humm, it seems this one is on Hold now....   :-|
Just remark: I retested based on todays v.49.0.2. Still same issue occuring.
Cheers.

Comment 13

[Mike Taylor [:miketaylr]]

> Humm, it seems this one is on Hold now....

:) Yes, our team is finishing up a work week, will come back to this soon. (Unless other people beat me to it.)

Comment 14

[Mike Taylor [:miketaylr]]

Thomas, can you look into this?

Comment 15

[Thomas Wisniewski]

Attachment: Attempt at a reduced test-case

The "blank" image tags mixed in the key part of markup are the culprits, as they affect the layout of their sibling divs differently in Chrome and Firefox:

> <div class="_1yk0 _1yk1">
>   <div>
>     <div class="_49r2  _52jd _52jb _52jg">
>       <img>
>       <div class="_49r3">Arts &amp; Entertainment</div>
>     </div>
>     <div class="_5cnj">
>       <div class="_49r2  _52jc _52j9 _52jg">
>         <img>
>         <div class="_49r3">Movie Theater, Museum, Sports Venue &amp; Stadium</div>
>       </div>
>     </div>
>   </div>
> </div>

I honestly don't know why though, as in a reduced test-case Chrome renders them the same way that Firefox does, and I haven't found the reason why they appear differently in the live example.

Regardless, there is a way to get the two engines rendering similarly:

  1. change the imgs to be display:block or none, perhaps in script or perhaps like this:
       ._49r2 img:not([src]) { display:none; }

  2. remove position:absolute from class _49r3


Note that I only tested on Chrome and Firefox desktop for now (both using mobile UAs). I tried the same with Safari, but I couldn't get to the actual page with the error; I couldn't click on the "hours" section to get to the page with the problem, and even appending "&crowd_view[id]=7" to the URL (which did the trick for Firefox) didn't get Safari to the page.

Hopefully this is at least a start...

Comment 16

[Mike Taylor [:miketaylr]]

(In reply to Thomas Wisniewski from comment #15)
> The "blank" image tags mixed in the key part of markup are the culprits, as
> they affect the layout of their sibling divs differently in Chrome and
> Firefox:

Hmm, interesting. I wonder if that behavior is defined in a spec somewhere? Let's at least move the component, this doesn't seem to be text selection at all.

Comment 17

[Mike Taylor [:miketaylr]]

(attempting a better title)

Comment 18

[Tobias K.]

Bug still existing in Android Firefox 50.0.2

Comment 19

[Boris Zbarsky [:bzbarsky]]

> as in a reduced test-case Chrome renders them the same way that Firefox does

Just to make sure, was the reduced testcase in standards mode?

Comment 20

[Thomas Wisniewski]

> Just to make sure, was the reduced testcase in standards mode?

No, good catch (I somehow forgot a doctype). The engines do in fact render the testcase differently in standards mode (as well as xhtml-mobile10.dtd, which Facebook uses on their m subdomain).

In Chrome, the presence of the first img tag (see comment 15) pushes the second line of text down, despite the img tag being empty. It seems as though the parent is forced to be as tall as its line-height when the img tag is there, even though none of its children are contributing to its height. (Giving the img itself a line-height does not influence the layout).

Firefox just ignores the empty img tag entirely, contributing no height to its parent (and neither does the position:absolute text/div).

Comment 21

[Tobias K.]

I checked by v51.0.2, it's still existing.

Comment 22

[Mike Taylor [:miketaylr]]

(removing [needsdiagnosis] from our triage queue, seems like Tom figured this out and uploaded a reduced TC)

Comment 23

[Tobias K.]

Issue still visible in v52

Comment 24

[Tobias K.]

...And still vissible in v52.0.1

Comment 25

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 26

[Daniel Holbert [:dholbert] (vacation until Jun 23)]

Attachment: testcase 2 (same as former "attempt at a reduced testcase" but now in standards mode)

(In reply to Thomas Wisniewski from comment #20)

Just to make sure, was the reduced testcase in standards mode?

No, good catch (I somehow forgot a doctype). The engines do in fact render
the testcase differently in standards mode

For reference, here's the same reduced testcase, now with a doctype added, to trigger standards-mode.

With this: I can reproduce the issue in Firefox 52 Nightlies, but I can't reproduce the issue in current Nightly (or in Chrome). So I think this has been fixed.

Comment 27

[Daniel Holbert [:dholbert] (vacation until Jun 23)]

Fix range:
INFO: First good revision: 39dba5141dd90c70a861299459d418d230148d9f (2018-11-10)
INFO: Last bad revision: d3d642b624886729636c3690a806f38b4d737731 (2018-11-09)
INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d3d642b624886729636c3690a806f38b4d737731&tochange=39dba5141dd90c70a861299459d418d230148d9f

Given that this bug involved img tags with no src attribute ("blank" image tags per comment 15), and given that the bug went away if those img elements were collapsed to zero height by tweaking their display, it's a pretty safe bet to say this was a version of bug 1196668 and was fixed by that bug's patch. Duping.