865515 - Rotating multi-pane fragments in Settings displays margins incorrectly

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Comment 1

[Chenxia Liu [:liuche]]

Was playing around with the new settings, and noticed that the margins are set based on what orientation the tablet is in when Settings is launched, not what orientation the Settings screen is displayed in. This means if Settings is launched in horizontal mode, the larger margin settings persist even when the tablet is rotated to vertical (this cuts off some text); vice versa is also true (with smaller margins persisting).

STR:
1. Have tablet in horizontal mode.
2. Launch Settings - margins are present on either side of the two panes to center the view.
3. Rotate tablet to vertical.

Expected:
Margins decrease to fit smaller horizontal space.

Actual:
Margins remain, even with smaller horizontal viewing space. This cramps the text (and truncates some titles).

Comment 2

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Vertical view margins from vertical launch

Comment 3

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Vertical view margins from horizontal launch

Comment 4

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Vertical view margins from vertical launch

(derp, rotated)

Comment 5

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Vertical view margins from horizontal launch

Comment 6

[Wesley Johnston (:wesj)]

We probably need to reinflate the content on rotations....

Comment 7

[Shilpan Bhagat]

Attachment: Patch: Preferences pane now sets correct layout on config change

Does as described above. Pretty straight forward.

Comment 8

[Chenxia Liu [:liuche]]

Comment on attachment 786732 [details] [diff] [review]
Patch: Preferences pane now sets correct layout on config change

Clearing the review flag because it turns out that the fix for this would be much more complicated than we first thought.

Removing "onConfigChange" does fix the margin bug, but regresses some other behavior [1]. We decided that there are higher priority bugs to focus on, especially in light of the gains (fixed margins) versus added code complexity of the correct fix.

[1] Removing onConfigChange causes recreation of the activity on all rotations, so we lose state for some dialogs that might be open (Clear private data, Master password, Import from Android, etc). If we ever move to having separate screens for these preferences (unlikely) this would be easier to fix.

The correct fix would be to implement View.onSaveInstanceState and View.onRestoreInstanceState, but to do this correctly involves handling saving some extra preference state. For instance, for Clear private data, there are a series of checkboxes whose state should be saved on rotation, on "confirm", but not on "cancel." One approach would be to save a separate preference for the checkbox state that gets cleared on "cancel" or "commit," but persists through config changes.

Comment 9

[Chenxia Liu [:liuche]]

[2] The other regression I forgot to mention is that the Fragment titles disappeared on rotation until a header is clicked.

Comment 10

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.