Open Bug 1308702 Opened 3 years ago Updated 7 months ago

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

Categories

(Core :: Layout, defect, P3)

All
Android
defect

Tracking

()

Tracking Status
firefox52 --- wontfix

People

(Reporter: aryx, Unassigned)

References

(Depends on 1 open bug)

Details

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
Assignee: nobody → bugmail
OS: Unspecified → Android
Hardware: Unspecified → All
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.
See Also: → 1281907
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.
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.
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.
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.
Mass wontfix for bugs affecting firefox 52.
See Also: → 1412774
Not actively working on this.
Assignee: bugmail → nobody
Depends on: 1504659

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.

Depends on: 1520320

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.

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

You need to log in before you can comment on or make changes to this bug.