Closed
Bug 632649
Opened 10 years ago
Closed 10 years ago
Warn users on devices with 256mb of RAM or less that they are on an unsupported device
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(fennec2.0b5+)
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0b5+ | --- |
People
(Reporter: pavlov, Assigned: blassey)
References
Details
(Whiteboard: [has-patch])
Attachments
(3 files, 1 obsolete file)
3.28 KB,
patch
|
dougt
:
review+
|
Details | Diff | Splinter Review |
66.07 KB,
image/png
|
mfinkle
:
ui-review+
|
Details |
3.73 KB,
patch
|
dougt
:
review+
|
Details | Diff | Splinter Review |
Can we in our Java code pop up a warning the first time a user runs that tells them something along the lines of: "Firefox does not currently support this device and you may experience problems."
Reporter | ||
Updated•10 years ago
|
tracking-fennec: --- → 2.0b5+
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → blassey.bugs
Comment 1•10 years ago
|
||
Adding Madhava for string inpupt
Updated•10 years ago
|
OS: All → Android
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #511165 -
Flags: review?(doug.turner)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #511166 -
Flags: ui-review?(madhava)
Comment 4•10 years ago
|
||
What will actually happen if they try to run it? Also - will we likely ever support it? If not, we should probably drop to: "Firefox does not support this device. You may experience problems." Now, it'd be great if we could be more specific or helpful about what a user is going to encounter, to help them make a real decision about whether to proceed. Do we know what these problems are?
Comment 5•10 years ago
|
||
Also - if the user does opt to "Continue", is he or she going to see this prompt every time on startup? If so, we should have a "[x] Show this on start up" in there.
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to comment #4) > What will actually happen if they try to run it? hangs and crashes > > Also - will we likely ever support it? If not, we should probably drop to: > nope > "Firefox does not support this device. You may experience problems." ok > > Now, it'd be great if we could be more specific or helpful about what a user is > going to encounter, to help them make a real decision about whether to proceed. > Do we know what these problems are? nothing damaging to their phone... hangs, crashes and slowness. Just setting expectations (In reply to comment #5) > Also - if the user does opt to "Continue", is he or she going to see this > prompt every time on startup? If so, we should have a "[x] Show this on start > up" in there. just after the first install and not again after that.
Updated•10 years ago
|
Whiteboard: [has-patch]
Comment 7•10 years ago
|
||
Comment on attachment 511166 [details]
screenshot
"Firefox does not support this device. You may experience problems."
r+ with the string change
Attachment #511166 -
Flags: ui-review?(madhava) → ui-review+
Comment 8•10 years ago
|
||
Comment on attachment 511165 [details] [diff] [review] patch > .setCancelable(false) >- .setPositiveButton(getResources().getString(R.string.exit_label), >+ .setPositiveButton(R.string.exit_label, I do not understand why we use to call getResources().getString(). > new DialogInterface.OnClickListener() { > public void onClick(DialogInterface dialog, > int id) > { > GeckoApp.this.finish(); >+ System.exit(0); comment why we are calling exit() here instead of just calling finish(). Also in the other exit() call.
Attachment #511165 -
Flags: review?(doug.turner) → review+
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to comment #8) > Comment on attachment 511165 [details] [diff] [review] > patch > > > > .setCancelable(false) > >- .setPositiveButton(getResources().getString(R.string.exit_label), > >+ .setPositiveButton(R.string.exit_label, > > I do not understand why we use to call getResources().getString(). You can either pass a resource id or a string. No need to get the string ourselves when the system can do it. > > > new DialogInterface.OnClickListener() { > > public void onClick(DialogInterface dialog, > > int id) > > { > > GeckoApp.this.finish(); > >+ System.exit(0); > > > comment why we are calling exit() here instead of just calling finish(). Also > in the other exit() call. We didn't need the exit() call prior to the network watcher landing, now we do
Comment 10•10 years ago
|
||
Could we add a link to https://www.mozilla.com/en-US/mobile/platforms/ so users can find more info?
Comment 11•10 years ago
|
||
(In reply to comment #10) > Could we add a link to https://www.mozilla.com/en-US/mobile/platforms/ so users > can find more info? Out of scope for this bug
Assignee | ||
Comment 12•10 years ago
|
||
pushed http://hg.mozilla.org/mozilla-central/rev/1f86b3c908cf
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 13•10 years ago
|
||
(In reply to comment #10) > Could we add a link to https://www.mozilla.com/en-US/mobile/platforms/ so users > can find more info? Thomas, can you file a followup bug to add this? I would like to see this in a future release (maybe even 4.0 if we can do it without new localizable strings).
Comment 14•10 years ago
|
||
Done: see Bug 633649
Comment 15•10 years ago
|
||
Aaron, can you please verify this fix against the Motorola Droid/Milestone that you have? looks like it alerts on firstrun. 2.12 +<!ENTITY incompatable_device "&brandShortName; does not support this device and you may experience 2.13 +problems."> 2.14 +<!ENTITY continue_label "Continue">
Comment 16•10 years ago
|
||
(In reply to comment #15) > Aaron, can you please verify this fix against the Motorola Droid/Milestone that > you have? looks like it alerts on firstrun. > > 2.12 +<!ENTITY incompatable_device "&brandShortName; does not support this > device and you may experience > 2.13 +problems."> > 2.14 +<!ENTITY continue_label "Continue"> Also, mbrubeck: tchung: only on first run, or after uninstall/reinstall or "Clear Data" (in the Android settings, not the Firefox prefs)
Comment 17•10 years ago
|
||
I am not seeing this warning notification. 1. I updated over 20110211, cleared data, started Fennec and saw nothing. 2. Ran a complete uninstall and clean install of 20110212, started Fennec and saw nothing. Mozilla/5.0 (Android; Linux armv7l; rv:2.0b12pre) Gecko/20110212 Firefox/4.0b12pre Fennec/4.0b5pre Device: Motorola Milestone Android: 2.2.1
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 18•10 years ago
|
||
Comment on attachment 511165 [details] [diff] [review] patch >+ // We don't currently support devices with less than 256Mb of RAM, warn on first run >+ if (Runtime.getRuntime().totalMemory() <= 262144L && !new File(sGREDir, "application.ini").exists()) { The documentation says that totalMemory returns a value in bytes, so (a) the threshold should be 268435456L, and (b) why isn't this triggering on all devices right now?
Assignee | ||
Comment 19•10 years ago
|
||
(In reply to comment #18) > Comment on attachment 511165 [details] [diff] [review] > patch > > >+ // We don't currently support devices with less than 256Mb of RAM, warn on first run > >+ if (Runtime.getRuntime().totalMemory() <= 262144L && !new File(sGREDir, "application.ini").exists()) { > > The documentation says that totalMemory returns a value in bytes, so (a) the > threshold should be 268435456L, and (b) why isn't this triggering on all > devices right now? huh? coulda sworn I read that it returned memory in kb. as for #b, good question.
Comment 20•10 years ago
|
||
(In reply to comment #18) > The documentation says that totalMemory returns a value in bytes, so (a) the > threshold should be 268435456L, and (b) why isn't this triggering on all > devices right now? Actually, ignore part (b) - I was thinking backwards for some reason. But it's true that totalMemory returns memory in bytes. I also don't think that totalMemory is what we want here. It's not the total physical memory of the host, but rather "the total amount of memory in the Java virtual machine. The value returned by this method may vary over time, depending on the host environment." In a small test app on my G2 (512 MB of physical RAM), it returns 2953200, or about 2.8 MB.
Comment 21•10 years ago
|
||
We could also implement this in C++ or JavaScript using PR_GetPhysicalMemorySize or nsSystemInfo (if that's any simpler than grepping /proc/meminfo in Java).
Comment 22•10 years ago
|
||
Is there nothing on the java side that give you the "MemTotal" from /proc/meminfo?
Comment 23•10 years ago
|
||
also, this should not block.
Assignee | ||
Comment 24•10 years ago
|
||
(In reply to comment #23) > also, this should not block. I disagree, we want this for b5
Comment 25•10 years ago
|
||
who is we? it is some little protection on a set of devices that we dont support. polish. like to have, but i find it hard to say id block all of the other b5 changes on it.
Assignee | ||
Comment 26•10 years ago
|
||
Attachment #512052 -
Flags: review?(doug.turner)
Comment 27•10 years ago
|
||
Comment on attachment 512052 [details] [diff] [review] patch to use /proc/meminfo Would you mind creating a new method called |IsSupportedDevice()| and move the logic to test for info there. This will allow us to just add other stuff (like testing for ARM6 or something like that) without having to refactor. So, then you can do something like: if (!IsSupportedDevice()) show_the_dialog... And the try and its indentation can be in that other method. Could you also add a comment why 256 is our limit? Doesn't have to be anything elborate, but it is pretty vague now. 256 is total system memory shared frame buffers, system, file cache... so not all 256mb devices will act the same. what is |propNames| for? Otherwise looks good.
Attachment #512052 -
Flags: review?(doug.turner) → review-
Assignee | ||
Comment 28•10 years ago
|
||
Attachment #512052 -
Attachment is obsolete: true
Attachment #512085 -
Flags: review?(doug.turner)
Comment 29•10 years ago
|
||
Comment on attachment 512085 [details] [diff] [review] patch to use /proc/meminfo remove |propNames|
Attachment #512085 -
Flags: review?(doug.turner) → review+
Assignee | ||
Comment 30•10 years ago
|
||
pushed http://hg.mozilla.org/mozilla-central/rev/385abb827ff3
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Comment 31•10 years ago
|
||
Mozilla/5.0 (Android; Linux armv7l; rv:2.0b12pre) Gecko/20110214 Firefox/4.0b12pre Fennec/4.0b5pre Verifying that I now see this with http://hg.mozilla.org/mozilla-central/rev/385abb827ff3 Device: Motorola Milestone Android: 2.2.1 Anyone else want to confirm on another currently unsupported device?
Comment 32•10 years ago
|
||
Also confirmed fixed, using today's nightly Fennec build on: Device: Motorola Milestone Android: 2.2.1
Updated•10 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•