Closed
Bug 752681
Opened 12 years ago
Closed 12 years ago
Galaxy Note and other mid-sized devices receive XUL Fennec's phone UI, instead of Native Fennec
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox13 fixed, firefox14 fixed, firefox-esr10 affected, blocking-fennec1.0 +)
RESOLVED
FIXED
Firefox 15
People
(Reporter: mbrubeck, Assigned: mbrubeck)
References
Details
Attachments
(1 file, 1 obsolete file)
3.11 KB,
patch
|
blassey
:
review+
mfinkle
:
approval-mozilla-aurora+
mfinkle
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
When we publish split builds (Native Fennec + XUL Fennec), our goal is to serve XUL Fennec only to devices that use the tablet UI. The tablet UI is enabled for screenSize="xlarge" devices, which includes 9- and 10-inch tablets. However, because of this patch from bug 719560, we will also serve XUL Fennec to screenSize="large" devices, which includes 7-inch tablets and some very large phones like the Samsung Galaxy Note. XUL Fennec runs with the phone layout on those devices: https://hg.mozilla.org/mozilla-central/rev/e3bb8af42f98 That patch was needed in order to install XUL Fennec on Tegra boards for testing, because the Tegras have "large" screens. (See bug 719560 comment 22.) But for the builds we ship to the market, we want XUL Fennec on "xlarge" devices only. So on mozilla-beta branch and mozilla-release, we need tinderbox builds that are Tegra-compatible and official release builds that are not. Since both types of builds currently use the same branding on Android, we can't do that just by editing the branding files. Is there some configure flag we can use? Any other suggestions?
Assignee | ||
Comment 1•12 years ago
|
||
I didn't notice that bug 752017 was already filed for a decision on whether we want to fix this bug.
Depends on: 752017
Assignee | ||
Comment 2•12 years ago
|
||
Currently it looks like the config differences between tinderbox builds and release builds are the --with-official-branding flag and the --enable-update-channel setting. This patch builds XUL fennec in xlarge-only mode only when --with-official-branding is enabled. If we'd rather have a more specific flag, we'd need to add it to the build automation.
Comment 3•12 years ago
|
||
The thought was in driver triage to block XUL Fennec on the Galaxy Note and it would fall back to native Fennec instead of changing the screen size targets.
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Kevin Brosnan [:kbrosnan] from comment #3) > The thought was in driver triage to block XUL Fennec on the Galaxy Note and > it would fall back to native Fennec instead of changing the screen size > targets. Is it possible to block devices on a per-APK basis? If so, that sounds fine. We should also block it for other screenSize="large" devices like the 7" Galaxy Tab.
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Kevin Brosnan [:kbrosnan] from comment #3) > The thought was in driver triage to block XUL Fennec on the Galaxy Note and > it would fall back to native Fennec instead of changing the screen size > targets. Based on my own testing with a split product in the Google Play Store, there's no way to block a device for just one APK. You can only exclude devices for the entire product (i.e. both APKs).
Comment 6•12 years ago
|
||
Comment on attachment 621744 [details] [diff] [review] patch Review of attachment 621744 [details] [diff] [review]: ----------------------------------------------------------------- this will result in the official builds failing to run tests. I think the right solution is to black list the note in the market for the xul build
Attachment #621744 -
Flags: review?(blassey.bugs) → review-
Assignee | ||
Comment 7•12 years ago
|
||
Comment on attachment 621744 [details] [diff] [review] patch (In reply to Brad Lassey [:blassey] from comment #6) > this will result in the official builds failing to run tests. Do we run automation on official builds? (Even if we do, as philor will tell you, no one checks the test results on the release branch anyway...) > I think the right solution is to black list the note in the market for the > xul build This is not possible. See comment 5.
Attachment #621744 -
Flags: review- → review?(blassey.bugs)
Comment 8•12 years ago
|
||
From bug 752017: "Native Phone UI on 5" devices for sure vs. the tablet UI. Might be slightly odd, but no odder than anything else on a 5" device. Definitely makes more sense that the 10" tablet UI."
blocking-fennec1.0: ? → +
Assignee | ||
Comment 9•12 years ago
|
||
(In reply to Matt Brubeck (:mbrubeck) from comment #7) > > this will result in the official builds failing to run tests. > > Do we run automation on official builds? As far as I can tell, we do *not* run automated tests on official builds.
Comment 10•12 years ago
|
||
Matt let me know that this will need to land on both MOBILE130_2012050119_RELBRANCH and tip of beta since it blocks our go to build for the XF13b2 spin associated with FN14b1. Would that be possible today?
Assignee | ||
Comment 11•12 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #10) > Matt let me know that this will need to land on both > MOBILE130_2012050119_RELBRANCH and tip of beta since it blocks our go to > build for the XF13b2 spin associated with FN14b1. Would that be possible > today? Yes, I can land this today if I get r+ today.
Assignee | ||
Comment 12•12 years ago
|
||
This switches to setting MOZ_MOBILE_COMPAT in the mozconfig, which seems a bit cleaner. Untested.
Attachment #622860 -
Flags: review?(blassey.bugs)
Assignee | ||
Comment 13•12 years ago
|
||
(In reply to Matt Brubeck (:mbrubeck) from comment #12) > This switches to setting MOZ_MOBILE_COMPAT in the mozconfig, which seems a > bit cleaner. Untested. I've now tested this approach successfully in local builds with and without MOZ_MOBILE_COMPAT in the mozconfig.
Assignee | ||
Comment 14•12 years ago
|
||
(In reply to Brad Lassey [:blassey] from comment #6) > this will result in the official builds failing to run tests. Aki confirmed that we do not run tests on official builds. (Well, he also thought it was possible that we run them but don't display the results anywhere, which amounts to the same thing.)
Assignee | ||
Updated•12 years ago
|
Attachment #621744 -
Attachment is obsolete: true
Attachment #621744 -
Flags: review?(blassey.bugs)
Comment 15•12 years ago
|
||
Comment on attachment 622860 [details] [diff] [review] alternate patch Review of attachment 622860 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/xul/config/mozconfigs/android/release @@ +17,5 @@ > ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL} > > export JAVA_HOME=/tools/jdk6 > export MOZILLA_OFFICIAL=1 > +export MOZ_MOBILE_COMPAT=Tablets why is this needed? shouldn't the configure.sh changes be sufficient?
Assignee | ||
Comment 16•12 years ago
|
||
(In reply to Brad Lassey [:blassey] from comment #15) > ::: mobile/xul/config/mozconfigs/android/release > > +export MOZ_MOBILE_COMPAT=Tablets > > why is this needed? shouldn't the configure.sh changes be sufficient? I'm moving this configuration from configure.sh to the mozconfig. Previously we set MOZ_MOBILE_COMPAT in configure.sh depending on the branding. With this patch, it defaults to "All" in configure.in, but in official builds the mozconfig explicitly sets it to "Tablets".
Updated•12 years ago
|
Attachment #622860 -
Flags: review?(blassey.bugs) → review+
Assignee | ||
Comment 17•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/672f5159bcdd
Assignee | ||
Comment 18•12 years ago
|
||
Comment on attachment 622860 [details] [diff] [review] alternate patch We need this patch on the mobile 13.0b2 relbranch for the next respin of XUL Fennec 13 beta. We will also need this on ESR 10 for the Native Fennec 1.0 final release, assuming we continue to serve XUL Fennec ESR 10 to tablet users on the release channel. I'll request esr10 approval after this has been released on beta. [Approval Request Comment] User impact if declined: Users of some devices receive XUL Fennec instead of Native Fennec. Testing completed (on m-c, etc.): Landed on m-i, but this patch only affects "official" beta and release builds, so it can't be tested in tinderbox builds or nightlies. I tested it in local builds that I uploaded to my own Google Play Store account. Risk to taking this patch (and alternatives if risky): This patch is XUL-Fennec-only, and it only changes the list of supported screen types in the Android manifest. It should be very low-risk. String or UUID changes made by this patch: None. See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #622860 -
Flags: approval-mozilla-beta?
Attachment #622860 -
Flags: approval-mozilla-aurora?
Comment 19•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/672f5159bcdd
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Attachment #622860 -
Flags: approval-mozilla-beta?
Attachment #622860 -
Flags: approval-mozilla-beta+
Attachment #622860 -
Flags: approval-mozilla-aurora?
Attachment #622860 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 20•12 years ago
|
||
Found a line that was left in by accident; removed it in a followup patch: https://hg.mozilla.org/integration/mozilla-inbound/rev/7b425021598e Landed on beta (including 13.0b2 relbranch) and aurora: https://hg.mozilla.org/releases/mozilla-beta/rev/88ee14110a74 https://hg.mozilla.org/releases/mozilla-beta/rev/fdf0d6f54fd4 https://hg.mozilla.org/releases/mozilla-aurora/rev/7bac62505304
Comment 21•12 years ago
|
||
(In reply to Matt Brubeck (:mbrubeck) from comment #20) > Found a line that was left in by accident; removed it in a followup patch: > https://hg.mozilla.org/integration/mozilla-inbound/rev/7b425021598e https://hg.mozilla.org/mozilla-central/rev/7b425021598e
You need to log in
before you can comment on or make changes to this bug.
Description
•