1308702 - Fix layout/reftests/bugs/1133905* so they work on Android

Status: NEW

(Core :: Layout, defect, P3)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Bug 1307719 switches the tests which use the layout/reftests/bugs/1133905* files to Android and disables the tests because they fail, see https://treeherder.mozilla.org/#/jobs?repo=try&revision=d64534f73a6dbfa911982b24433984c282922f1d

Comment 1

[Kartikaya Gupta (email:[e-mail])]

So this appears to be an actual bug. When I load -1-v.html and -ref-v.html in the browser on my Z3C, I see the vertical scrollbar in -ref-v.html but not in -1-v.html.

Possibly also related to bug 1281907.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

I ran mozregression to find out when -1-v.html started rendering without the scrollbar, and it looks like it's been that way ever since we switched from the Java PZC to the C++ one - that's also when we switched from Java scrollbars to the Gecko ones, so it makes sense.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

I looked at the frame and display list dumps of the two pages, -1-v.html and -ref-v.html to see if I could figure out what was going on. The frame dump for both pages is more or less what I would expect:

-1-v.html (which has a meta-viewport width=325) has something like this:

HTMLScroll(html)(-1)@8f1fed88 {0,0,19500,28113}
  ...
  ScrollbarFrame(scrollbar)(-1)@905beac8 next=905bf648 {21240,0,360,31141}

Here the HTMLScroll is 325 CSS pixels wide and the scrollbar frame is positioned at x=354 (so that it is right-aligned to a device-width of 360 CSS pixels).

-ref-v.html has a meta-viewport of width=735 and looks like this:

HTMLScroll(html)(-1)@90e57d88 {0,0,43500,62713}
  ...
  ScrollbarFrame(scrollbar)(-1)@92d99ac8 next=92d9a648 {21240,0,360,31140}

So the scrollbar is positioned in the same place, but the HTMLScroll frame has a width of 725 CSS pixels.

When it comes to the display list dump, things are more different. The -1-v.html page has this item:

OwnLayer p=0x92d805c8 f=0x905beac8(ScrollbarFrame(scrollbar)(-1)) z=2147483647 bounds(0,0,0,0) layerBounds(0,0,0,0) visible(21240,0,360,31141) componentAlpha(0,0,0,0) clip(0,0,19500,28113) scrollClip()

while the -ref-v.html page has this item:

OwnLayer p=0x94e545c8 f=0x92d99ac8(ScrollbarFrame(scrollbar)(-1)) z=2147483647 bounds(21240,0,120,1500) layerBounds(21240,0,120,1500) visible(21240,0,360,31140) componentAlpha(0,0,0,0) clip(0,0,43500,62713) scrollClip()

We can see that in the -1-v.html the clip rect is smaller, matching the HTMLScroll bounds, and the bounds of the scrollbar item are 0, presumably because it gets clipped out.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

The stuff in comment 2 and comment 3 seems unrelated to the reftest failure, I'll file another bug for that.

The reftest failure seems to be (entirely?) a result of the apz.allow_zooming pref being false in Android reftests. Once bug 1307719 makes it to m-c, I'll update the reftest.list file to flip that pref to true for the relevant tests, and audit the fuzzy checks to make sure they're appropriate.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6ba3e831ba3355b155fcbaa247278aee547d438

Comment 6

[Kartikaya Gupta (email:[e-mail])]

The failures in the try push above show that the reference pages have no scrollbars. It's only when the meta-viewport width goes above some threshold value that the scrollbars show up. So the stuff in comment 2 and comment 3 might still be relevant here. I'll keep digging. The reason I was seeing -1-v.html pass locally with the pref set was that both the test and reference pages had no scrollbars. We should probably guard against that by adding a != about:blank test as well.

Comment 7

[Julien Cristau [:jcristau]]

Mass wontfix for bugs affecting firefox 52.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Not actively working on this.

Comment 9

[Brad Werth [:bradwerth]]

As Bug 1504659 lands, it fixes some of these tests, and causes others to fail. It also regresses tests for Bug 1242172, probably for similar reasons.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

One of the failure reason is that those reftests assume that the content is zoomed in/out automatically, but unfortunately we do explicitly set apz.allow_zooming false. So we need to set the pref for those tests explicitly. Anyway with my patch for bug 1520077 those tests start failing (the vertical scroll bar is rendered with my patch, I believe it's properly rendered as far as I can see the result on my android phone), so I am going to handle them in bug 1520077.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #10)

One of the failure reason is that those reftests assume that the content is zoomed in/out automatically, but unfortunately we do explicitly set apz.allow_zooming false. So we need to set the pref for those tests explicitly. Anyway with my patch for bug 1520077 those tests start failing (the vertical scroll bar is rendered with my patch, I believe it's properly rendered as far as I can see the result on my android phone), so I am going to handle them in bug 1520077.

And we should also use reftest-async-zoom there as Markus suggested in bug 1133905 comment 27.