710297 - Canvas becomes clipped and unpannable

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Henri Sivonen (:hsivonen)]

(I used Galaxy Nexus with Android 4.0.1. Dunno if that matters.)

Steps to reproduce:
 1) Navigate to http://www.hs.fi/
 2) Click the title/lede/picture of the first story to navigate to a story page.
 3) Double-tap to zoom.
 4) Try to pan.
 5) Multi-touch zoom.
 6) Try to pan.
 7) Retry a couple of times. If you don't see a problem, quit Fennec and try again.

Actual results:
Sometimes, the canvas gets clipped to the area that fits on the screen. After that happens, trying to pan doesn't work. The view behaves as if the area that fits on the screen is all there is. That is, the canvas just snaps back. When zooming out, the view makes it look like the area was clipped, but at least in the most recent build, the clip region seems to update after zooming.

Expected results:
Expected the view to act as if the bounds of the canvas were the bounds of the content. Expected no clipping to an area smaller than that. Expected to be able to pan to all the content at any given zoom level.

Comment 1

[Aaron Train [:aaronmt]]

This is a pretty bad issue with the browser that should block any release.

Comment 3

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

This looks like it was introduced by the patch for bug 707935. The problem is the article pages on hs.fi stay in document.readyState == "loading" for a very long time, so we don't update the page size until much later. The throbber even stops, giving the appearance that the page has stopped loading, even though it isn't - if you wait longer eventually the throbber comes back, the page finishes loading for real, and we update the page size. The throbber may or may not be fixed with bug 712414, but regardless, we should do periodic updates to the page size.

Comment 4

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

Attachment: Send page size updates more often

Comment 6

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

QA: please also verify bug 712596 when verifying this bug.

Comment 9

[Chris Lord [:cwiiis]]

Comment on attachment 583610 [details] [diff] [review]
Send page size updates more often

Review of attachment 583610 [details] [diff] [review]:
-----------------------------------------------------------------

r+ from me with a lengthier comment for the below code.

::: mobile/android/chrome/content/browser.js
@@ +1344,5 @@
> +       * this causes the page size to jump around wildly during page load.
> +       */
> +      if (doc.readyState === 'complete' || (pageWidth >= gScreenWidth && pageHeight >= gScreenHeight)) {
> +        this._viewport.pageWidth = pageWidth;
> +        this._viewport.pageHeight = pageHeight;

Took me a few seconds to realise that this is what we want to do due to the zoom behaviour. It's worth adding a comment about us wanting page sizes less than the screen after loading so that we can adjust the zoom for the page to fit.

Comment 10

[Patrick Walton (:pcwalton)]

Comment on attachment 583610 [details] [diff] [review]
Send page size updates more often

Review of attachment 583610 [details] [diff] [review]:
-----------------------------------------------------------------

Fine by me. Note that patch part 5 in bug 709492 kills the readyState check entirely, since it's no longer necessary at that point to prevent page size from jumping around.

Comment 11

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

Landed on m-i:

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb41941cdf9d

Comment 12

[Geoff Lankow (:darktrojan)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b58836d3fa78

Backed out on suspicion of causing Android m1 and m2 orange.

Comment 13

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

Comment on attachment 583610 [details] [diff] [review]
Send page size updates more often

Investigating oranges

Comment 14

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

Re-landed on m-i: https://hg.mozilla.org/integration/mozilla-inbound/rev/bfe14712fe3c
Try run ran green: https://tbpl.mozilla.org/?tree=Try&rev=30bbb4acc8ca

Oranges were caused by the patch for bug 712761

Comment 15

[Matt Brubeck (:mbrubeck)]

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

Comment 16

[LegNeato]

Comment on attachment 583610 [details] [diff] [review]
Send page size updates more often

[triage comment]
Approved for aurora. QA says this is bad and would block a release.

Comment 17

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

Landed on aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/7986e3f95244

Comment 18

[Aaron Train [:aaronmt]]

Samsung Nexus S (Android 4.0.3)
Samsung Galaxy SII (Android 2.3.4)
20111230040819
http://hg.mozilla.org/mozilla-central/rev/9fdaea5d67e2
20111229042021
http://hg.mozilla.org/releases/mozilla-aurora/rev/06335a118a5a

Comment 20

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

Fixing target milestone I incorrectly changed when landing on aurora.

Comment 21

[Geoffrey D.]

Can confirm there is still something going on with firefox 29 for android.

Got an "Untrusted Connection" warning with no possibility to see the "add exception" button, which is very similar to bug 712596. I cannot pan or zoom.

Comment 22

[Geoffrey D.]

Apologies, after using the remote debugger I found out that was a normal behaviour:

- I am behind a proxy that requires authentication and it expired.

- when going to an http page a authentication prompt is displayed but with an https url, only the untrusted connection is displayed

- the Exception section is hidden by default since the error is "badStsCert". The page js contains a comment stating:
// Disallow overrides if this is a Strict-Transport-Security
// host and the cert is bad (STS Spec section 7.3) or if the
// certerror is in a frame (bug 633691).