Closed
Bug 734382
Opened 12 years ago
Closed 12 years ago
Don't show the list of activities if there is only one
Categories
(Core Graveyard :: Widget: Android, defect)
Tracking
(firefox13 fixed, firefox14 fixed)
RESOLVED
FIXED
mozilla14
People
(Reporter: mounir, Assigned: mounir)
References
Details
Attachments
(1 file, 1 obsolete file)
3.54 KB,
patch
|
blassey
:
review+
|
Details | Diff | Splinter Review |
Follow-up from bug 730289. Quite simple actually.
Attachment #604404 -
Flags: review?(doug.turner)
Comment 1•12 years ago
|
||
Comment on attachment 604404 [details] [diff] [review] Patch v1 Review of attachment 604404 [details] [diff] [review]: ----------------------------------------------------------------- brad requested this follow.
Attachment #604404 -
Flags: review?(doug.turner) → review?(blassey.bugs)
Comment 2•12 years ago
|
||
Comment on attachment 604404 [details] [diff] [review] Patch v1 Review of attachment 604404 [details] [diff] [review]: ----------------------------------------------------------------- code looks good, but I'd like to see a new patch with the refactor I describe below. ::: mobile/android/base/GeckoApp.java @@ +2485,5 @@ > + } > + > + intent = intents.get(itemId); > + } else { > + intent = intents.get(0); my preference would be to factor this code out into a helper function, getFilePickerIntent(String mimeType) In that helper function, 0 and 1 possible intents can be early returns.
Attachment #604404 -
Flags: review?(blassey.bugs) → review-
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #604404 -
Attachment is obsolete: true
Attachment #605714 -
Flags: review?(blassey.bugs)
Comment 4•12 years ago
|
||
Comment on attachment 605714 [details] [diff] [review] Patch v2 Review of attachment 605714 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/config/check-sync-dirs.py @@ +1,1 @@ > +#!/usr/bin/env python2 I have no idea if there is some js style guide against this. Probably easiest just to drop this change. ::: mobile/android/base/GeckoApp.java @@ +2459,5 @@ > ArrayList<Intent> intents = new ArrayList<Intent>(); > PromptService.PromptListItem[] items = getItemsAndIntentsForFilePicker(aMimeType, intents); > > + if (intents.size() == 0) { > + Log.e(LOGTAG, "no activities for the file picker!"); this isn't really an error. Log.i would be better @@ +2465,5 @@ > + } > + > + if (intents.size() == 1) { > + return intents.get(0); > + } nit, no curly braces for a one line if statement
Attachment #605714 -
Flags: review?(blassey.bugs) → review+
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Brad Lassey [:blassey] from comment #4) > ::: js/src/config/check-sync-dirs.py > @@ +1,1 @@ > > +#!/usr/bin/env python2 > > I have no idea if there is some js style guide against this. Probably > easiest just to drop this change. That thing had nothing to do in that patch actually. Sorry and thank you for noticing ;)
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7e8278f5e814
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Comment 7•12 years ago
|
||
I backed this out along with all the other patches in the initial push. Something in the push regressed Ts on Android by 20% and I don't know which patch is to blame: https://hg.mozilla.org/mozilla-central/rev/e94edfdb1f5b Regression Ts increase 20.6% on Android 2.2 Mozilla-Inbound -------------------------------------------------------------- Previous: avg 2653.656 stddev 87.856 of 30 runs up to revision 1239bd0779a6 New : avg 3201.178 stddev 110.202 of 5 runs since revision 0d61a0d8dba4 Change : +547.522 (20.6% / z=6.232) Graph : http://mzl.la/zD3EWy
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c47e0e62d6d0
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 9•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/ec0e723221ae
status-firefox13:
--- → fixed
status-firefox14:
--- → fixed
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•