1304394 - [iOS 10] Menu layout is broken when restoring Firefox from background

Status: VERIFIED FIXED

(Firefox for iOS :: General, defect, P1)

Description

[Simon Basca [:SimonB]]

Attachment: IMG_0050.PNG

Build: 5.3(1) RC
Device: iPad Air 2
iOS: 10.0.1

Steps to reproduce:
1. Launch Firefox in portrait view
2. Open the menu
3. Background Firefox by pressing the home button 
4. Restore Firefox

Actual results;
- The menu layout will be broken and all the icons will overlap 

Expected:
- The menu should be correctly displayed

Note:
- This issue was encountered on iPad Air 2 and iPad mini 4 running iOS 10.

- This issue was not reproduced on 5.2(1) released version.

Comment 1

[Catalin Suciu [:csuciu]]

This is a major UI regression affecting all iPads with iOS 10 and it should be fixed in the 5.3 release - if it's not too late

Comment 2

[Aaron Train [:aaronmt]]

Not reproducible on my iPhone 6 (iOS 10). Seems tablet specific.

Comment 3

[Farhan Patel [:farhan]]

I looked into this a bit more today. This issue can be fixed by closing the menu and reopening it.

Sadly, I'm having trouble understanding why exactly this is happening. I'm writing my thoughts here for when I want to get back to this. 

A change in the menuViewController's appState when going into the background triggers a layout. That triggers a full reloadData of the Menu which causes "prepareLayout" in the Menu's colletionViewLayout. Turns out the bounds of the collectionView are incorrect (width=0). But the layout continues anyways and fails horribly (see screenshot).

This happens because the constraints for the width aren't set the same way they are for the height. One way of fixing this. Is to fix the width of the popOver. This way the case of the collectionview ever being 0 never happens. 

This all wouldn't be such a big deal if once the app appeared again viewDidAppear was triggered and layout happened again. But it isn't

Comment 4

[Stephan Leroux [:sleroux]]

Thanks for looking into this. This seems related to the iOS 10 bug that was fixed a few weeks back where the menu wasn't correctly laying out at all on iPads [1]. From what I can recall, on iOS 10, the UI

> Turns out the bounds of the collectionView are incorrect (width=0). But the layout continues anyways and fails horribly (see screenshot).

My gut says that the timing of the reloadData which in turn forces the relayout is occurring at an incorrect time in the view lifecycle. For example, if you try laying things out manually without autolayout prior to viewDidLayoutSubviews, you get some undefined frame values.

[1] https://github.com/mozilla-mobile/firefox-ios/pull/2070

Comment 5

[Farhan Patel [:farhan]]

Attachment: Pull Request

Comment 6

[Stephan Leroux [:sleroux]]

Comment on attachment 8798004 [details] [review]
Pull Request

Looks good!

Comment 7

[Farhan Patel [:farhan]]

master https://github.com/mozilla-mobile/firefox-ios/commit/d78c4790d57597f0f6354150a11a13be68c4ec6d

Comment 8

[Catalin Suciu [:csuciu]]

Verifying as fixed on master acc25e6d9.

The Menu is dismissed when foregrounding Firefox.