1136363 - Viewport sometimes doesn't resize correctly (and resize event doesn't fire) when phone is rotated, in Firefox Android

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:
 1. Load attached testcase in Nightly on Android.
 2. Dismiss any initial alert dialogs.
 3. Rotate your phone back & forth between Landscape & Portrait.

EXPECTED RESULTS:
An alert should pop up when you rotate your phone, to indicate that the viewport has resized.

ACTUAL RESULTS:
No alert.


ALTERNATE STR:
 1. Load about:firefox in portrait mode.
 2. Rotate phone to landscape mode.

EXPECTED RESULTS: Page fills new viewport.

ACTUAL RESULTS: Right half of viewport is blank, until I touch screen, at which point the non-blank section scales up & awkwardly fills viewport like I've got a really-tall, zoomed-in portrait view.



Firefox Beta (v36) -- installed from the Play store today -- gives EXPECTED RESULTS -- alerts show up, and about:firefox rotates nicely.

Firefox Nightly (39.0a1 2015-02-24) gives ACTUAL RESULTS. No alerts on rotation, and about:firefox loads with weird blankness-followed-by-scaling as noted above.

Comment 1

[Daniel Holbert [:dholbert]]

I can't reproduce the bug in Firefox Nightly on Desktop, btw (in responsive design mode, clicking "rotate" button). I get a bunch of alerts for each of our incremental resizes there.

I've only hit this in Nightly on Android.

kats, do you know if this is a known issue, and/or if there's anyone else who should be CC'd /needinfo'd here?

Comment 2

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

We've had issues like this in the past but I don't think anything has changed between Beta and Nightly. And I'm not able to reproduce on my Nightly build on a Nexus 4 using the test case you attached. I get a dialog on initial load as well as on every rotation. about:firefox also seems to resize as I would expect on rotation. What device are you using?

Comment 3

[Daniel Holbert [:dholbert]]

I get EXPECTED RESULTS in latest Aurora (37, 2015-02-23), FWIW.

It's possible there's something profile-dependent here, too (i.e. my Nightly profile might be busted somehow).

I'm using a OnePlus One.

Comment 4

[Daniel Holbert [:dholbert]]

Darn -- I force-quit Nightly (using the app-switcher) and reopened it, and now I'm no longer hitting the bug.

So I guess that means this is sporadic & may only happen in long-running sessions, or after some yet-to-be-isolated step is performed.

Comment 5

[Paul [pwd]]

Just came to report this, it's been doing my head in.

Comment 6

[Daniel Holbert [:dholbert]]

Just to be sure it's the same issue -- does the problem go away after you force-quit firefox (by swiping it offscreen in the app-switcher)?

(There may not be much we can do here until we've discovered semi-reliable steps that trigger this issue.  If you can figure any out, please mention them!)

Comment 7

[Paul [pwd]]

Nope, 100% reproducible on today's build

1. Go to mmangareader.me
2. Rotate phone
3. tap blank space to force resizing

Comment 8

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

STR from comment 7 WFM on a nexus 4 with nightly 2015-03-08. Paul, can you grab a logcat from when you rotate?

Comment 9

[Paul [pwd]]

Attachment: tmp_2383-2015-03-09-19-27-481583567190.txt

Here you go Kats.

Comment 10

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

I see this in the log on a rotation:

[JavaScript Error: "TypeError: metadata is undefined" {file: "chrome://browser/content/browser.js" line: 4668}]

That's probably causing the problem, but I'm not sure why that's happening.

Comment 11

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

As far as I can tell this should never happen. The metadata local variable is assigned at [1], and that value comes from [2] which is implemented at [3]. Even if for whatever reason there isn't a metadata in the WeakMap it should return a new empty metadata object. Somebody who can reproduce this would have to look at what's going on in the remote debugger to figure it out. I don't know if you're able to do that.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=7697ad4919e7#4667
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=7697ad4919e7#4617
[3] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=7697ad4919e7#6426

Comment 12

[Paul [pwd]]

If you tell exactly what to do, I'm happy to provide whatever data you require.

Comment 13

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

I ran into this undefined metadata error while trying to run reftests on Fennec. I added some logging and the first time it hit it was on a valid document passed in to getMetadataForDocument, so as far as I can tell it's a WeakMap bug of some sort.

03-11 15:48:06.694 I/Gecko   (17136): nsWindow: 0x84425600 OnSizeChanged [768 1038]
03-11 15:48:06.704 D/GeckoBrowser(17136): returning metadata [undefined] for document [[object HTMLDocument]]
03-11 15:48:06.714 W/GeckoConsole(17136): [JavaScript Error: "TypeError: metadata is undefined" {file: "chrome://browser/content/browser.js" line: 4668}]

However we can probably work around it in browser.js.

Comment 14

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

Attachment: Patch

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

Kats - It looks like this was changed in bug 1127827 which landed in fx38. Can you request uplift once this patch lands?

Comment 16

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

Ah, thanks for tracking that down.

Comment 17

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

https://hg.mozilla.org/integration/fx-team/rev/2c2b67d059d9

Comment 18

[Andrew McCreight [:mccr8]]

Sorry about that.  I tried to audit all of the uses of WeakMap in the tree, but I guess I missed a place where it actually mattered.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2c2b67d059d9

Comment 20

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

Comment on attachment 8576212 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: bug 1127827
[User impact if declined]: sometimes rotation gets broken in that after rotation the viewport is messed up and content may not get resize events as expected.
[Describe test coverage new/current, TreeHerder]: i was able to repro under somewhat contrived conditions and verified it was fixed there. not sure if we have STR for real-world scenarios that encounter this bug.
[Risks and why]: pretty low risk, fennec-only change
[String/UUID change made/needed]: none

Comment 21

[Paul [pwd]]

FYI, this isn't fixed in the 0316 build, I'll try again in the 0317 build but as per the STR comment 7, this is still 100% reproducible right now.

Comment 22

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

The change didn't make it into the 0316 build. You can try the m-c build at http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-central-android-api-11/1426588411/ which should have it, or wait until tomorrow's nightly.

Comment 23

[Paul [pwd]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #22)
> The change didn't make it into the 0316 build. You can try the m-c build at
> http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-
> central-android-api-11/1426588411/ which should have it, or wait until
> tomorrow's nightly.

I waited for 0317, tested and it's still not fixed.

Comment 24

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

Can you provide the changeset URL from about:buildconfig of the build you tested on?

Comment 25

[Paul [pwd]]

Built from https://hg.mozilla.org/mozilla-central/rev/436686833af0

Comment 26

[Lukas Blakk [:lsblakk] use ?needinfo]

triage drive-by, waiting to approve uplift until there's confirmation of the fix on nightly.

Comment 27

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

(In reply to Paul [pwd] from comment #25)
> Built from https://hg.mozilla.org/mozilla-central/rev/436686833af0

That build doesn't have the fix. Please try on tomorrow's nightly, or the build I linked in comment 22.

Comment 28

[Paul [pwd]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #27)
> (In reply to Paul [pwd] from comment #25)
> > Built from https://hg.mozilla.org/mozilla-central/rev/436686833af0
> 
> That build doesn't have the fix. Please try on tomorrow's nightly, or the
> build I linked in comment 22.

Ooh it's good to have it working properly again. I can indeed verify that the build in comment 22 doesn't feature this bug.

Comment 29

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

Great, thanks!

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ead1bd9d09b0