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)

All
Android
defect
Not set
normal

Tracking

(firefox13 fixed, firefox14 fixed, firefox-esr10 affected, blocking-fennec1.0 +)

RESOLVED FIXED
Firefox 15
Tracking Status
firefox13 --- fixed
firefox14 --- fixed
firefox-esr10 --- affected
blocking-fennec1.0 --- +

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

Attachments

(1 file, 1 obsolete file)

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?
I didn't notice that bug 752017 was already filed for a decision on whether we want to fix this bug.
Depends on: 752017
Attached patch patch (obsolete) — Splinter Review
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.
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Attachment #621744 - Flags: review?(blassey.bugs)
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.
(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.
(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 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-
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)
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: ? → +
(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.
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?
(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.
Attached patch alternate patchSplinter Review
This switches to setting MOZ_MOBILE_COMPAT in the mozconfig, which seems a bit cleaner.  Untested.
Attachment #622860 - Flags: review?(blassey.bugs)
(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.
(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.)
Attachment #621744 - Attachment is obsolete: true
Attachment #621744 - Flags: review?(blassey.bugs)
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?
(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".
Attachment #622860 - Flags: review?(blassey.bugs) → review+
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?
https://hg.mozilla.org/mozilla-central/rev/672f5159bcdd
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Attachment #622860 - Flags: approval-mozilla-beta?
Attachment #622860 - Flags: approval-mozilla-beta+
Attachment #622860 - Flags: approval-mozilla-aurora?
Attachment #622860 - Flags: approval-mozilla-aurora+
(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.

Attachment

General

Created:
Updated:
Size: