Last Comment Bug 712477 - [ICS] Landscape resources are not used in landscape mode
: [ICS] Landscape resources are not used in landscape mode
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: -- normal (vote)
: Firefox 12
Assigned To: Sriram Ramasubramanian [:sriram]
:
Mentors:
Depends on: 712808
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-20 15:42 PST by Sriram Ramasubramanian [:sriram]
Modified: 2012-01-10 14:14 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Patch (5.67 KB, patch)
2011-12-21 00:07 PST, Sriram Ramasubramanian [:sriram]
mark.finkle: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review

Description Sriram Ramasubramanian [:sriram] 2011-12-20 15:42:28 PST
ICS has separate resources specified for landscape mode. However, they aren't used by Android.
Comment 1 Sriram Ramasubramanian [:sriram] 2011-12-20 15:44:06 PST
From my primary investigation, if the app is launched in landscape mode (holding the device landscape mode), the landscape specific resources are used. However, they are not used when rotating from portrait mode. This is because the views aren't redrawn during the orientation change.
Comment 2 Matt Brubeck (:mbrubeck) 2011-12-20 17:23:36 PST
(In reply to Sriram Ramasubramanian [:sriram] from comment #1)
> From my primary investigation, if the app is launched in landscape mode
> (holding the device landscape mode), the landscape specific resources are
> used. However, they are not used when rotating from portrait mode. This is
> because the views aren't redrawn during the orientation change.

Is this because we have android:configChanges="orientation" in AndroidManifest.xml?
Comment 3 Sriram Ramasubramanian [:sriram] 2011-12-20 22:24:16 PST
We need to track "orientation" to avoid restarting activity. However, in onConfigurationChange(), we should redraw the layout for landscape. Which doesn't seem to work.
Comment 4 Sriram Ramasubramanian [:sriram] 2011-12-21 00:07:28 PST
Created attachment 583412 [details] [diff] [review]
Patch

This patch solves the issue (phew!)
Ideally invalidate() should do the trick. However, it is not taking the new configuration into account while redrawing.
I had to inflate the mBrowserToolbar again. Since we have a different landscape view only for ICS, this is done only for honeycomb+ devices (as the action bar is changed).
The orientation has to be saved to compare against, as inside onConfigurationChanged(), getConfiguration().orientation will be same as newConfig.orientation.

I found few files (drawable-land-v14/ and drawable-port-v14/) not needed. I have removed them in this patch.
Comment 5 Mark Finkle (:mfinkle) (use needinfo?) 2011-12-21 13:53:59 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/23681bd07e4a
Comment 6 Ed Morley [:emorley] 2011-12-22 03:50:52 PST
https://hg.mozilla.org/mozilla-central/rev/23681bd07e4a
Comment 7 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-06 22:57:35 PST
Comment on attachment 583412 [details] [diff] [review]
Patch

[Approval Request Comment]
User impact if declined: We use the wrong assests in the UI
Testing completed (on m-c, etc.): It's been on m-c for a while
Risk to taking this patch (and alternatives if risky): Low risk
Comment 8 Alex Keybl [:akeybl] 2012-01-09 14:54:10 PST
Comment on attachment 583412 [details] [diff] [review]
Patch

[Triage Comment]
Mobile only - approved for Aurora.
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-10 14:14:04 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/3e4decfc3bb3

Note You need to log in before you can comment on or make changes to this bug.