749953 - panning (scrolling) does not work correctly on Android XUL nightly builds and wikipedia home page displays incorrectly

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Bill Gianopoulos [:WG9s]]

I see this on a Samsung Galaxy Tab 8.9 running Honeycomb 3.2

While viewing http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/ I am unable to pan to the bottom of the page in landscape mode, yet can pan way past the bottom of the page if in Portrait.

hg bisect determined the following:

The first bad revision is:
changeset:   92047:e9053acaa5e1
user:        Jeff Muizelaar <[e-mail]>
date:        Thu Apr 12 15:55:26 2012 -0400
summary:     Bug 744901 - Store the page size in FrameMetrics in CSS pixels in addition to device pixels. r=cjones

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: Revert a hunk

Comment 2

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 619306
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=3cbb8707c98d
Try run started, revision 3cbb8707c98d. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=3cbb8707c98d

Comment 3

[Bill Gianopoulos [:WG9s]]

That will fix it.  I already proved that.  I was going to post a workaround patch that reverted that part only on XUL builds.

Comment 4

[Bill Gianopoulos [:WG9s]]

OK.  So, This patch results in both XUL and native android builds being able to correctly pan http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/ correctly.

Comment 5

[Mozilla RelEng Bot]

Try run for 3cbb8707c98d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3cbb8707c98d
Results (out of 15 total builds):
    success: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-3cbb8707c98d

Comment 6

[Bill Gianopoulos [:WG9s]]

Just a stupid question, but what does "autoland-in-queue" mean? seeing as how this is something I am not familiar with, and this does not appear to have landed anywhere.

Comment 7

[Bill Gianopoulos [:WG9s]]

OIC.  That has to do with the try build.  Nervermind!

Comment 8

[Bill Gianopoulos [:WG9s]]

This issue seems to also break the display of http://en.wikipedia.org/wiki/Main_Page setting to top50 site broken.

Comment 9

[Bill Gianopoulos [:WG9s]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #1)
> Created attachment 619306 [details] [diff] [review]
> Revert a hunk

So, what is the status of this patch?  I notice you have neither taken the bug nor asked for review.  Is this on a track to land, or are you waiting to see my kludgey patch first?

Comment 10

[Bill Gianopoulos [:WG9s]]

Changing severity to blocker.  It is is impossible to test Fennec XUL nightly builds with this issue present because it causes so many things to render incorrectly.

Comment 11

[Matt Brubeck (:mbrubeck)]

Requesting tracking-firefox14 because this is a bad regression in XUL Fennec 14.

Comment 13

[Matt Brubeck (:mbrubeck)]

Jeff, can this patch get reviewed and landed?  This is a blocker for the XUL Fennec 14 beta release.

Comment 14

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 619306 [details] [diff] [review]
Revert a hunk

Revert a hunk from 744901 that causes regressions for reasons I don't understand.

Comment 15

[Robert Kaiser]

Even if this ended up not blocking anything due to us continuing to ship 13.0b2 to tablets for now, we should give the people using XUL Aurora or Nightly on tablets a decent experience, esp. if it's a simple backout of a hunk that fixes this and should not make other stuff worse. Also, we should have the choice to actually ship 14 on the beta channel to tablets once it hits the beta repo - not saying that we will or have plans, but we should enable the choice for release managers if it's easy.

So, ping on the review?

Comment 16

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 619306 [details] [diff] [review]
Revert a hunk

*shrug*

Comment 17

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/804afa6bce8e

Comment 18

[Jeff Muizelaar [:jrmuizel]]

Looks like we don't store the device pixels content size anymore...

Comment 19

[Jeff Muizelaar [:jrmuizel]]

We need to use the rect, backed out for now:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7b4bd185f18

Comment 20

[Bill Gianopoulos [:WG9s]]

OK so now that backing out no longer works, the implications of this regression are much more severe.

Comment 21

[Bill Gianopoulos [:WG9s]]

But then I have no idea how to make it more severe than blocker.

Comment 23

[Jeff Muizelaar [:jrmuizel]]

Lets try again:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb4ba09133ab

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/eb4ba09133ab

Comment 25

[Robert Kaiser]

From what I see in some testing on my tablet, this indeed fixed it. Could we have that for 14 as well, please?

Comment 27

[Lukas Blakk [:lsblakk] use ?needinfo]

We're never going to put out a 14 XUL Fennec, so marking status as wontfix.

Comment 28

[Matt Brubeck (:mbrubeck)]

(In reply to Lukas Blakk [:lsblakk] from comment #27)
> We're never going to put out a 14 XUL Fennec, so marking status as wontfix.

In the planning meeting yesterday, it was stated that:
> XUL Fennec 14 beta 5 will be released as a security update for beta tablet users on 6/7
https://wiki.mozilla.org/Firefox/Planning/2012-05-30

See also the diagram: http://cl.ly/2A2T3u1v2Y3C0M1X230e

Are we no longer planning to release XF 14 beta?

Comment 29

[Lukas Blakk [:lsblakk] use ?needinfo]

Yes, we'll have XF beta, sorry for the lack of clarity in my earlier statement. If we want this on the beta channel (never to be released) then please go ahead and nominate for aurora approval.

Comment 30

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 619306 [details] [diff] [review]
Revert a hunk

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 744901 
User impact if declined: panning is broken on Android XUL nightly builds
Testing completed (on m-c, etc.): been on m-c for a bit
Risk to taking this patch (and alternatives if risky): This just reverts part of 744901 the risk should be pretty low.
String or UUID changes made by this patch: None

Comment 31

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 619306 [details] [diff] [review]
Revert a hunk

Looks good, low risk and bake time on m-c, approving.

Comment 32

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3bd9d0c544e0